Skip to content

Commit

Permalink
fixup! GenericErrorConstructor adaptor
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Feb 11, 2024
1 parent 0c98c96 commit ca5b2a4
Show file tree
Hide file tree
Showing 10 changed files with 220 additions and 64 deletions.
16 changes: 13 additions & 3 deletions packages/common/throw-labeled.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,24 @@ import { X, makeError, annotateError } from '@endo/errors';
*
* @param {Error} innerErr
* @param {string|number} label
* @param {ErrorConstructor=} ErrorConstructor
* @param {import('ses/src/error/types.js').GenericErrorConstructor} [errConstructor]
* @param {import('ses/src/error/types.js').AssertMakeErrorOptions} [options]
* @returns {never}
*/
export const throwLabeled = (innerErr, label, ErrorConstructor = undefined) => {
export const throwLabeled = (
innerErr,
label,
errConstructor = undefined,
options = undefined,
) => {
if (typeof label === 'number') {
label = `[${label}]`;
}
const outerErr = makeError(`${label}: ${innerErr.message}`, ErrorConstructor);
const outerErr = makeError(
`${label}: ${innerErr.message}`,
errConstructor,
options,
);
annotateError(outerErr, X`Caused by ${innerErr}`);
throw outerErr;
};
Expand Down
4 changes: 2 additions & 2 deletions packages/errors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ const {
} = globalAssert;
/** @type {import("ses").AssertionFunctions } */
// @ts-expect-error missing properties assigned next
const assert = (value, optDetails, optErrorContructor) =>
globalAssert(value, optDetails, optErrorContructor);
const assert = (value, optDetails, errContructor, options) =>
globalAssert(value, optDetails, errContructor, options);
Object.assign(assert, assertions);

export {
Expand Down
5 changes: 3 additions & 2 deletions packages/marshal/src/marshal-justin.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,9 @@ const decodeToJustin = (encoding, shouldIndent = false, slots = []) => {
}
case 'error': {
const { name, message } = rawTree;
typeof name === 'string' ||
Fail`invalid error name typeof ${q(typeof name)}`;
if (typeof name !== 'string') {
throw Fail`invalid error name typeof ${q(typeof name)}`;
}
getErrorConstructor(name) !== undefined ||
Fail`Must be the name of an Error constructor ${name}`;
typeof message === 'string' ||
Expand Down
39 changes: 31 additions & 8 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ export const makeMarshal = (
assert.typeof(message, 'string');
const name = encodeRecur(`${err.name}`);
assert.typeof(name, 'string');
// Must encode `cause`, `errors`.
// nested non-passable errors must be ok from here.
// TODO Must encode `cause`, `errors`, but
// only once all possible counterparty decoders are tolerant of
// receiving them.
if (errorTagging === 'on') {
// We deliberately do not share the stack, but it would
// be useful to log the stack locally so someone who has
Expand Down Expand Up @@ -254,12 +255,25 @@ export const makeMarshal = (
};

/**
* @param {{errorId?: string, message: string, name: string}} errData
* @param {{
* errorId?: string,
* message: string,
* name: string,
* cause: unknown,
* errors: unknown,
* }} errData
* @param {(e: unknown) => Passable} decodeRecur
* @returns {Error}
*/
const decodeErrorCommon = (errData, decodeRecur) => {
const { errorId = undefined, message, name, ...rest } = errData;
const {
errorId = undefined,
message,
name,
cause = undefined,
errors = undefined,
...rest
} = errData;
ownKeys(rest).length === 0 ||
Fail`unexpected encoded error properties ${q(ownKeys(rest))}`;
// TODO Must decode `cause` and `errors` properties
Expand All @@ -272,13 +286,22 @@ export const makeMarshal = (
Fail`invalid error name typeof ${q(typeof dName)}`;
typeof dMessage === 'string' ||
Fail`invalid error message typeof ${q(typeof dMessage)}`;
const EC = getErrorConstructor(dName) || Error;
const errConstructor = getErrorConstructor(dName) || Error;
// errorId is a late addition so be tolerant of its absence.
const errorName =
dErrorId === undefined
? `Remote${EC.name}`
: `Remote${EC.name}(${dErrorId})`;
const error = makeError(dMessage, EC, { errorName });
? `Remote${errConstructor.name}`
: `Remote${errConstructor.name}(${dErrorId})`;
const options = {
errorName,
};
if (cause) {
options.cause = decodeRecur(cause);
}
if (errors) {
options.errors = decodeRecur(errors);
}
const error = makeError(dMessage, errConstructor, options);
return harden(error);
};

Expand Down
61 changes: 52 additions & 9 deletions packages/pass-style/src/error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="ses"/>

import { X, Fail, annotateError } from '@endo/errors';
import { X, Fail, annotateError, makeError } from '@endo/errors';
import { assertChecker } from './passStyle-helpers.js';

/** @typedef {import('./internal-types.js').PassStyleHelper} PassStyleHelper */
Expand All @@ -14,7 +14,7 @@ const { ownKeys } = Reflect;
const errorConstructors = new Map(
// Cast because otherwise TS is confused by AggregateError
// See https://github.com/endojs/endo/pull/2042#discussion_r1484933028
/** @type {Array<[string, ErrorConstructor]>} */
/** @type {Array<[string, import('ses/src/error/types.js').GenericErrorConstructor]>} */
([
['Error', Error],
['EvalError', EvalError],
Expand All @@ -29,6 +29,16 @@ const errorConstructors = new Map(
]),
);

/**
* Because the error constructor returned by this function might be
* `AggregateError`, which has different construction parameters
* from the other error constructors, do not use it directly to try
* to make an error instance. Rather, use `makeError` which encapsulates
* this non-uniformity.
*
* @param {string} name
* @returns {import('ses/src/error/types.js').GenericErrorConstructor | undefined}
*/
export const getErrorConstructor = name => errorConstructors.get(name);
harden(getErrorConstructor);

Expand All @@ -46,6 +56,7 @@ const checkErrorLike = (candidate, check = undefined) => {
);
};
harden(checkErrorLike);
/// <reference types="ses"/>

/**
* Validating error objects are passable raises a tension between security
Expand Down Expand Up @@ -76,18 +87,20 @@ export const ErrorHelper = harden({

canBeValid: checkErrorLike,

assertValid: candidate => {
assertValid: (candidate, passStyleOfRecur) => {
ErrorHelper.canBeValid(candidate, assertChecker);
const proto = getPrototypeOf(candidate);
const { name } = proto;
const EC = getErrorConstructor(name);
(EC && EC.prototype === proto) ||
const errConstructor = getErrorConstructor(name);
(errConstructor && errConstructor.prototype === proto) ||
Fail`Errors must inherit from an error class .prototype ${candidate}`;

const {
// TODO Must allow `cause`, `errors`
message: mDesc,
stack: stackDesc,
cause: causeDesc = undefined,
errors: errorsDesc = undefined,
...restDescs
} = getOwnPropertyDescriptors(candidate);
ownKeys(restDescs).length < 1 ||
Expand All @@ -104,6 +117,22 @@ export const ErrorHelper = harden({
!stackDesc.enumerable ||
Fail`Passed Error "stack" ${stackDesc} must not be enumerable`;
}
if (causeDesc) {
ErrorHelper.assertValid(causeDesc.value, passStyleOfRecur);
!causeDesc.enumerable ||
Fail`Passed Error "cause" ${causeDesc} must not be enumerable`;
}
if (errorsDesc) {
const errors = errorsDesc.value;
passStyleOfRecur(errors) === 'copyArray' ||
Fail`Passed Error "errors" must be an array`;
for (const subErr of errors) {
ErrorHelper.assertValid(subErr, passStyleOfRecur);
}
!errorsDesc.enumerable ||
Fail`Passed Error "errors" ${errorsDesc} must not be enumerable`;
}

return true;
},
});
Expand All @@ -112,14 +141,28 @@ export const ErrorHelper = harden({
* Return a new passable error that propagates the diagnostic info of the
* original, and is linked to the original as a note.
*
* @param {Error} err
* @param {Error | AggregateError} err
* @returns {Error}
*/
export const toPassableError = err => {
const { name, message } = err;
const {
name,
message,
cause = undefined,
// @ts-expect-error err might be an AggregateError, which would
// have an `errors` property. In addition, we tolerate `errors` on
// other errors, just like we do `cause`.
errors = undefined,
} = err;

const EC = getErrorConstructor(`${name}`) || Error;
const newError = harden(new EC(`${message}`));
const errConstructor = getErrorConstructor(`${name}`) || Error;
const newError = harden(
makeError(`${message}`, errConstructor, {
// @ts-ignore Assuming cause is Error | undefined
cause,
errors,
}),
);
// Even the cleaned up error copy, if sent to the console, should
// cause hidden diagnostic information of the original error
// to be logged.
Expand Down
23 changes: 23 additions & 0 deletions packages/pass-style/test/test-extended-errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/* eslint-disable max-classes-per-file */
import { test } from './prepare-test-env-ava.js';

// eslint-disable-next-line import/order
import { passStyleOf } from '../src/passStyleOf.js';

test('style of extended errors', t => {
const e1 = Error('e1');
t.throws(() => passStyleOf(e1), {
message: 'Cannot pass non-frozen objects like "[Error: e1]". Use harden()',
});
harden(e1);
t.is(passStyleOf(e1), 'error');

const e2 = harden(Error('e2', { cause: e1 }));
t.is(passStyleOf(e2), 'error');

const u3 = harden(URIError('u3', { cause: e1 }));
t.is(passStyleOf(u3), 'error');

const a4 = harden(AggregateError([e2, u3], 'a4', { cause: e1 }));
t.is(passStyleOf(a4), 'error');
});
1 change: 1 addition & 0 deletions packages/ses/src/commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const {
ReferenceError,
SyntaxError,
TypeError,
AggregateError,
} = globalThis;

export const {
Expand Down
43 changes: 34 additions & 9 deletions packages/ses/src/error/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
arrayPush,
assign,
freeze,
defineProperty,
globalThis,
is,
isError,
Expand All @@ -33,6 +34,7 @@ import {
weakmapGet,
weakmapHas,
weakmapSet,
AggregateError,
} from '../commons.js';
import { an, bestEffortStringify } from './stringify-utils.js';
import './types.js';
Expand Down Expand Up @@ -257,8 +259,8 @@ const tagError = (err, optErrorName = err.name) => {
*/
const makeError = (
optDetails = redactedDetails`Assert failed`,
ErrorConstructor = globalThis.Error,
{ errorName = undefined } = {},
errConstructor = globalThis.Error,
{ errorName = undefined, cause = undefined, errors = undefined } = {},
) => {
if (typeof optDetails === 'string') {
// If it is a string, use it as the literal part of the template so
Expand All @@ -270,7 +272,26 @@ const makeError = (
throw TypeError(`unrecognized details ${quote(optDetails)}`);
}
const messageString = getMessageString(hiddenDetails);
const error = new ErrorConstructor(messageString);
const opts = cause && { cause };
let error;
if (errConstructor === AggregateError) {
error = AggregateError(errors || [], messageString, opts);
} else {
error = /** @type {ErrorConstructor} */ (errConstructor)(
messageString,
opts,
);
if (errors !== undefined) {
// Since we need to tolerate `errors` on an AggregateError, may as
// well tolerate it on all errors.
defineProperty(error, 'errors', {
value: errors,
writable: true,
enumerable: false,
configurable: true,
});
}
}
weakmapSet(hiddenMessageLogArgs, error, getLogArgs(hiddenDetails));
if (errorName !== undefined) {
tagError(error, errorName);
Expand Down Expand Up @@ -382,9 +403,10 @@ const makeAssert = (optRaise = undefined, unredacted = false) => {
/** @type {AssertFail} */
const fail = (
optDetails = assertFailedDetails,
ErrorConstructor = globalThis.Error,
errConstructor = undefined,
options = undefined,
) => {
const reason = makeError(optDetails, ErrorConstructor);
const reason = makeError(optDetails, errConstructor, options);
if (optRaise !== undefined) {
optRaise(reason);
}
Expand All @@ -402,22 +424,25 @@ const makeAssert = (optRaise = undefined, unredacted = false) => {
function baseAssert(
flag,
optDetails = undefined,
ErrorConstructor = undefined,
errConstructor = undefined,
options = undefined,
) {
flag || fail(optDetails, ErrorConstructor);
flag || fail(optDetails, errConstructor, options);
}

/** @type {AssertEqual} */
const equal = (
actual,
expected,
optDetails = undefined,
ErrorConstructor = undefined,
errConstructor = undefined,
options = undefined,
) => {
is(actual, expected) ||
fail(
optDetails || details`Expected ${actual} is same as ${expected}`,
ErrorConstructor || RangeError,
errConstructor || RangeError,
options,
);
};
freeze(equal);
Expand Down
Loading

0 comments on commit ca5b2a4

Please sign in to comment.