Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ses,pass-style,marshal): permit Promise.any, AggregateError, cause #2042

Merged
merged 2 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/common/NEWS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
User-visible changes in `@endo/common`:

# Next release

- `throwLabeled` parameterized error construction
- Like the assertion functions/methods that were parameterized by an error
constructor (`makeError`, `assert`, `assert.fail`, `assert.equal`),
`throwLabeled` now also accepts named options `cause` and `errors` in its
immediately succeeding `options` argument.
- Like those assertion functions, the error constructor argument to
`throwLabeled` can now be an `AggregateError`.
If `throwLabeled` makes an error instance, it encapsulates the
non-uniformity of the `AggregateError` construction arguments, allowing
all the error constructors to be used polymorphically
(generic / interchangeable).
- The error constructor argument is now typed `GenericErrorConstructor`,
effectively the common supertype of `ErrorConstructor` and
`AggregateErrorConstructor`.
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').GenericErrorConstructor} [errConstructor]
* @param {import('ses').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: 4 additions & 0 deletions packages/common/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{
"extends": "../../tsconfig.eslint-base.json",
"compilerOptions": {
"checkJs": true,
"maxNodeModuleJsDepth": 1,
},
"include": [
"*.js",
"*.ts",
Expand Down
20 changes: 20 additions & 0 deletions packages/errors/NEWS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
User-visible changes in `@endo/errors`:

# Next release

- `AggegateError` support
- Assertion functions/methods that were parameterized by an error constructor
(`makeError`, `assert`, `assert.fail`, `assert.equal`) now also accept named
options `cause` and `errors` in their immediately succeeding
`options` argument.
- For all those, the error constructor can now be an `AggregateError`.
If they do make an error instance, they encapsulate the
non-uniformity of the `AggregateError` construction arguments, allowing
all the error constructors to be used polymorphically
(generic / interchangeable).
- Adds a `GenericErrorConstructor` type to be effectively the common supertype
of `ErrorConstructor` and `AggregateErrorConstructor`, for typing these
error constructor parameters that handle the error constructor
polymorphically.
- The SES `console` now includes `error.cause` and `error.errors` in
its diagnostic output for errors.
4 changes: 2 additions & 2 deletions packages/errors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,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);

// As of 2024-02, the Agoric chain's bootstrap vat runs with a version of SES
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/lib/configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ module.exports = {
lockdown: 'readonly',
harden: 'readonly',
HandledPromise: 'readonly',
// https://github.com/endojs/endo/issues/550
AggregateError: 'readonly',
},
rules: {
'@endo/assert-fail-as-throw': 'error',
Expand Down
26 changes: 26 additions & 0 deletions packages/marshal/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
User-visible changes in `@endo/marshal`:

# Next release

- Sending and receiving extended errors.
- As of the previous release, `@endo/marshal` tolerates extra error
properties with `Passable` values. However, all those extra properties
were only recorded in annotations, since they are not recognized as
legitimate on `Passable` errors.
- This release will use these extra properties to construct an error object
with all those extra properties, and then call `toPassableError` to make
the locally `Passable` error that it returns. Thus, if the extra properties
received are not recognized as a legitimate part of a locally `Passable`
error, the error with those extra properties itself becomes the annotation
on the returned `Passable` error.
- An `error.cause` property whose value is a `Passable` error with therefore
show up on the returned `Passable` error. If it is any other `Passable`
value, it will show up on the internal error used to annotate the
returned error.
- An `error.errors` property whose value is a `CopyArray` of `Passable`
errors will likewise show up on the returned `Passable` error. Otherwise,
only on the internal error annotation of the returned error.
- Although this release does otherwise support the error properties
`error.cause` and `error.errors` on `Passable` errors, it still does not
send these properties because releases prior to the previous release
do not tolerate receiving them. Once we no longer need to support
releases prior to the previous release, then we can start sending these.

# v1.2.0 (2024-02-14)

- Tolerates receiving extra error properties (https://github.com/endojs/endo/pull/2052). Once pervasive, this tolerance will eventually enable additional error properties to be sent. The motivating examples are the JavaScript standard properties `cause` and `errors`. This change also enables smoother interoperation with other languages with their own theories about diagnostic information to be included in errors.
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
64 changes: 46 additions & 18 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getInterfaceOf,
getErrorConstructor,
hasOwnPropertyOf,
toPassableError,
} from '@endo/pass-style';

import { X, Fail, q, makeError, annotateError } from '@endo/errors';
Expand All @@ -30,6 +31,7 @@ import {
/** @typedef {import('./types.js').Encoding} Encoding */
/** @typedef {import('@endo/pass-style').RemotableObject} Remotable */

const { defineProperties } = Object;
const { isArray } = Array;
const { ownKeys } = Reflect;

Expand Down Expand Up @@ -113,8 +115,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,40 +258,65 @@ 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;
// TODO Must decode `cause` and `errors` properties.
const {
errorId = undefined,
message,
name,
cause = undefined,
errors = undefined,
...rest
} = errData;
// 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 });
if (ownKeys(rest).length >= 1) {
// Note that this does not decodeRecur rest's property names.
// This would be inconsistent with smallcaps' expected handling,
// but is fine here since it is only used for `annotateError`,
// which is for diagnostic info that is otherwise unobservable.
const extras = objectMap(rest, decodeRecur);
annotateError(error, X`extra marshalled properties ${extras}`);
? `Remote${errConstructor.name}`
: `Remote${errConstructor.name}(${dErrorId})`;
const options = {
errorName,
};
if (cause) {
options.cause = decodeRecur(cause);
}
if (errors) {
options.errors = decodeRecur(errors);
}
return harden(error);
const rawError = makeError(dMessage, errConstructor, options);
// Note that this does not decodeRecur rest's property names.
// This would be inconsistent with smallcaps' expected handling,
// but is fine here since it is only used for `annotateError`,
// which is for diagnostic info that is otherwise unobservable.
const descs = objectMap(rest, data => ({
value: decodeRecur(data),
writable: false,
enumerable: false,
configurable: false,
}));
defineProperties(rawError, descs);
harden(rawError);
return toPassableError(rawError);
};

// The current encoding does not give the decoder enough into to distinguish
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
48 changes: 40 additions & 8 deletions packages/marshal/test/test-marshal-capdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,29 +154,28 @@ test('unserialize errors', t => {
});

test('unserialize extended errors', t => {
if (typeof AggregateError === 'undefined') {
t.pass('skip test on platforms prior to AggregateError');
return;
}
Comment on lines +157 to +160
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems too disruptive; we should only skip the narrow subset of test logic that won't work as expected.

const { unserialize } = makeTestMarshal();
const uns = body => unserialize({ body, slots: [] });

// TODO cause, errors, and AggregateError will eventually be recognized.
// See https://github.com/endojs/endo/pull/2042

const refErr = uns(
'{"@qclass":"error","message":"msg","name":"ReferenceError","extraProp":"foo","cause":"bar","errors":["zip","zap"]}',
);
t.is(getPrototypeOf(refErr), ReferenceError.prototype); // direct instance of
t.false('extraProp' in refErr);
t.false('cause' in refErr);
t.false('errors' in refErr);
console.log('error with extra prop', refErr);

const aggErr = uns(
'{"@qclass":"error","message":"msg","name":"AggregateError","extraProp":"foo","cause":"bar","errors":["zip","zap"]}',
);
t.is(getPrototypeOf(aggErr), Error.prototype); // direct instance of
t.is(getPrototypeOf(aggErr), AggregateError.prototype); // direct instance of
t.false('extraProp' in aggErr);
t.false('cause' in aggErr);
t.false('errors' in aggErr);
console.log('error with extra prop', aggErr);
t.is(aggErr.errors.length, 0);

const unkErr = uns(
'{"@qclass":"error","message":"msg","name":"UnknownError","extraProp":"foo","cause":"bar","errors":["zip","zap"]}',
Expand All @@ -185,7 +184,40 @@ test('unserialize extended errors', t => {
t.false('extraProp' in unkErr);
t.false('cause' in unkErr);
t.false('errors' in unkErr);
console.log('error with extra prop', unkErr);
});

const testIfAggregateError =
typeof AggregateError !== 'undefined' ? test : test.skip;

testIfAggregateError('unserialize errors w recognized extensions', t => {
const { unserialize } = makeTestMarshal();
const uns = body => unserialize({ body, slots: [] });

const errEnc = '{"@qclass":"error","message":"msg","name":"URIError"}';

const refErr = uns(
`{"@qclass":"error","message":"msg","name":"ReferenceError","extraProp":"foo","cause":${errEnc},"errors":[${errEnc}]}`,
);
t.is(getPrototypeOf(refErr), ReferenceError.prototype); // direct instance of
t.false('extraProp' in refErr);
t.is(getPrototypeOf(refErr.cause), URIError.prototype);
t.is(getPrototypeOf(refErr.errors[0]), URIError.prototype);

const aggErr = uns(
`{"@qclass":"error","message":"msg","name":"AggregateError","extraProp":"foo","cause":${errEnc},"errors":[${errEnc}]}`,
);
t.is(getPrototypeOf(aggErr), AggregateError.prototype); // direct instance of
t.false('extraProp' in aggErr);
t.is(getPrototypeOf(aggErr.cause), URIError.prototype);
t.is(getPrototypeOf(aggErr.errors[0]), URIError.prototype);

const unkErr = uns(
`{"@qclass":"error","message":"msg","name":"UnknownError","extraProp":"foo","cause":${errEnc},"errors":[${errEnc}]}`,
);
t.is(getPrototypeOf(unkErr), Error.prototype); // direct instance of
t.false('extraProp' in unkErr);
t.is(getPrototypeOf(unkErr.cause), URIError.prototype);
t.is(getPrototypeOf(unkErr.errors[0]), URIError.prototype);
});

test('passStyleOf null is "null"', t => {
Expand Down
Loading
Loading