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 d5473d8 commit f508532
Show file tree
Hide file tree
Showing 11 changed files with 234 additions and 69 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
18 changes: 13 additions & 5 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 Expand Up @@ -389,11 +390,18 @@ const decodeToJustin = (encoding, shouldIndent = false, slots = []) => {
}

case 'error': {
const { name, message } = rawTree;
// TODO cause, errors, AggregateError
// See https://github.com/endojs/endo/pull/2052
const {
name,
message,
cause = undefined,
errors = undefined,
} = rawTree;
cause === undefined ||
Fail`error cause not yet implemented in marshal-justin`;
name !== `AggregateError` ||
Fail`AggregateError not yet implemented in marshal-justin`;
errors === undefined ||
Fail`error errors not yet implemented in marshal-justin`;
return out.next(`${name}(${quote(message)})`);
}

Expand Down
41 changes: 32 additions & 9 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,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 @@ -255,31 +256,53 @@ 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;
// TODO Must decode `cause` and `errors` properties.
// See https://github.com/endojs/endo/pull/2052
// capData does not transform strings. The immediately following calls
// to `decodeRecur` are for reuse by other encodings that do,
// such as smallcaps.
const dName = decodeRecur(name);
const dMessage = decodeRecur(message);
// errorId is a late addition so be tolerant of its absence.
const dErrorId = errorId && decodeRecur(errorId);
typeof dName === 'string' ||
Fail`invalid error name typeof ${q(typeof dName)}`;
typeof dMessage === 'string' ||
Fail`invalid error message typeof ${q(typeof dMessage)}`;
const EC = getErrorConstructor(dName) || Error;
// errorId is a late addition so be tolerant of its absence.
const errConstructor = getErrorConstructor(dName) || Error;
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);
if (ownKeys(rest).length >= 1) {
// Note that this does not decodeRecur rest's property names.
// This would be inconsistent with smallcaps' expected handling,
Expand Down
4 changes: 3 additions & 1 deletion packages/marshal/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ export {};
* EncodingClass<'symbol'> & { name: string } |
* EncodingClass<'error'> & { name: string,
* message: string,
* errorId?: string
* errorId?: string,
* cause?: Encoding,
* errors?: Encoding[],
* } |
* EncodingClass<'slot'> & { index: number,
* iface?: string
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
Loading

0 comments on commit f508532

Please sign in to comment.