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

fix(ses,pass-style,marshal): fix #2428 permit disposal intrinsics #2429

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
4 changes: 4 additions & 0 deletions packages/common/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
User-visible changes in `@endo/common`:

# Next release

- TODO explain further generalization of `throwLabeled`

# v1.1.0 (2024-02-22)

- `throwLabeled` parameterized error construction
Expand Down
5 changes: 5 additions & 0 deletions packages/errors/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
User-visible changes in `@endo/errors`:

# Next release

- TODO explain `SuppressedError` support


# v1.1.0 (2024-02-22)

- `AggegateError` support
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 @@ -63,6 +63,8 @@ module.exports = {
HandledPromise: 'readonly',
// https://github.com/endojs/endo/issues/550
AggregateError: 'readonly',
// https://github.com/tc39/proposal-explicit-resource-management
SuppressedError: 'readonly',
},
rules: {
'@endo/assert-fail-as-throw': 'error',
Expand Down
4 changes: 4 additions & 0 deletions packages/marshal/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
User-visible changes in `@endo/marshal`:

# Next release

TODO explain `SuppressedError` support

# v1.5.1 (2024-07-30)

- `deeplyFulfilled` moved from @endo/marshal to @endo/pass-style. @endo/marshal still reexports it, to avoid breaking old importers. But importers should be upgraded to import `deeplyFulfilled` directly from @endo/pass-style.
Expand Down
2 changes: 2 additions & 0 deletions packages/marshal/src/marshal-justin.js
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,10 @@ const decodeToJustin = (encoding, shouldIndent = false, slots = []) => {
Fail`error cause not yet implemented in marshal-justin`;
name !== `AggregateError` ||
Fail`AggregateError not yet implemented in marshal-justin`;
// TODO SuppressedError
errors === undefined ||
Fail`error errors not yet implemented in marshal-justin`;
// TODO error,suppressed
return out.next(`${name}(${quote(message)})`);
}

Expand Down
16 changes: 13 additions & 3 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export const makeMarshal = (
assert.typeof(message, 'string');
const name = encodeRecur(`${err.name}`);
assert.typeof(name, 'string');
// TODO Must encode `cause`, `errors`, but
// TODO Must encode `cause`,`errors`,`error`,`suppressed` but
// only once all possible counterparty decoders are tolerant of
// receiving them.
if (errorTagging === 'on') {
Expand Down Expand Up @@ -262,8 +262,10 @@ export const makeMarshal = (
* errorId?: string,
* message: string,
* name: string,
* cause: unknown,
* errors: unknown,
* cause?: unknown,
* errors?: unknown,
* error?: unknown,
* suppressed?: unknown,
* }} errData
* @param {(e: unknown) => Passable} decodeRecur
* @returns {Error}
Expand All @@ -275,6 +277,8 @@ export const makeMarshal = (
name,
cause = undefined,
errors = undefined,
error = undefined,
suppressed = undefined,
...rest
} = errData;
// See https://github.com/endojs/endo/pull/2052
Expand Down Expand Up @@ -306,6 +310,12 @@ export const makeMarshal = (
if (errors) {
options.errors = decodeRecur(errors);
}
if (error) {
options.error = decodeRecur(error);
}
if (suppressed) {
options.suppressed = decodeRecur(suppressed);
}
const rawError = makeError(dMessage, errConstructor, options);
// Note that this does not decodeRecur rest's property names.
// This would be inconsistent with smallcaps' expected handling,
Expand Down
2 changes: 2 additions & 0 deletions packages/marshal/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export {};
* errorId?: string,
* cause?: Encoding,
* errors?: Encoding[],
* error?: Encoding,
* suppressed?: Encoding,
* } |
* EncodingClass<'slot'> & { index: number,
* iface?: string
Expand Down
2 changes: 2 additions & 0 deletions packages/marshal/test/marshal-capdata.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ testIfAggregateError('unserialize errors w recognized extensions', t => {
t.is(getPrototypeOf(unkErr.errors[0]), URIError.prototype);
});

// TODO SuppressedError

test('passStyleOf null is "null"', t => {
t.assert(passStyleOf(null), 'null');
});
Expand Down
2 changes: 2 additions & 0 deletions packages/marshal/test/marshal-smallcaps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ test('smallcaps unserialize errors w recognized extensions', t => {
t.is(getPrototypeOf(refErr.errors[0]), URIError.prototype);
});

// TODO SuppressedError

test('smallcaps mal-formed @qclass', t => {
const { unserialize } = makeTestMarshal();
const uns = body => unserialize({ body, slots: [] });
Expand Down
4 changes: 4 additions & 0 deletions packages/pass-style/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
User-visible changes in `@endo/pass-style`:

# Next release

TODO explain `SuppressedError` support

# v1.4.1 (2024-07-30)

- `deeplyFulfilled` moved from @endo/marshal to @endo/pass-style. @endo/marshal still reexports it, to avoid breaking old importers. But importers should be upgraded to import `deeplyFulfilled` directly from @endo/pass-style.
Expand Down
14 changes: 12 additions & 2 deletions packages/pass-style/src/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const errorConstructors = new Map(
// To accommodate platforms prior to AggregateError, we comment out the
// following line and instead conditionally add it to the map below.
// ['AggregateError', AggregateError],
// Likewise https://github.com/tc39/proposal-explicit-resource-management
// ['SuppressedError', SuppressedError],
]),
);

Expand All @@ -35,9 +37,15 @@ if (typeof AggregateError !== 'undefined') {
errorConstructors.set('AggregateError', AggregateError);
}

if (typeof SuppressedError !== 'undefined') {
// Conditional, to accommodate platforms prior to SuppressedError
errorConstructors.set('SuppressedError', SuppressedError);
}

/**
* Because the error constructor returned by this function might be
* `AggregateError`, which has different construction parameters
* `AggregateError` or `SuppressedError`,
* each of 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.
Expand Down Expand Up @@ -125,7 +133,9 @@ export const checkRecursivelyPassableErrorPropertyDesc = (
)} own property must be a string: ${value}`)
);
}
case 'cause': {
case 'cause':
case 'error':
case 'suppressed': {
// eslint-disable-next-line no-use-before-define
return checkRecursivelyPassableError(value, passStyleOfRecur, check);
}
Expand Down
21 changes: 19 additions & 2 deletions packages/pass-style/src/passStyleOf.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,22 +281,39 @@ export const toPassableError = err => {
return err;
}
const { name, message } = err;
const { cause: causeDesc, errors: errorsDesc } =
getOwnPropertyDescriptors(err);
const {
cause: causeDesc,
errors: errorsDesc,
error: errorDesc,
suppressed: suppressedDesc,
} = getOwnPropertyDescriptors(err);
let cause;
let errors;
let error;
let suppressed;
if (causeDesc && isPassableErrorPropertyDesc('cause', causeDesc)) {
cause = causeDesc.value;
}
if (errorsDesc && isPassableErrorPropertyDesc('errors', errorsDesc)) {
errors = errorsDesc.value;
}
if (errorDesc && isPassableErrorPropertyDesc('error', errorDesc)) {
error = errorDesc.value;
}
if (
suppressedDesc &&
isPassableErrorPropertyDesc('suppressed', suppressedDesc)
) {
suppressed = suppressedDesc.value;
}

const errConstructor = getErrorConstructor(`${name}`) || Error;
const newError = makeError(`${message}`, errConstructor, {
// @ts-ignore Assuming cause is Error | undefined
cause,
errors,
error,
suppressed,
});
// Still needed, because `makeError` only does a shallow freeze.
harden(newError);
Expand Down
5 changes: 5 additions & 0 deletions packages/pass-style/test/errors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ test('style of extended errors', t => {
const a4 = harden(AggregateError([e2, u3], 'a4', { cause: e1 }));
t.is(passStyleOf(a4), 'error');
}
if (typeof SuppressedError !== 'undefined') {
// Conditional, to accommodate platforms prior to SuppressedError
const a4 = harden(SuppressedError(e2, u3, 'a4'));
t.is(passStyleOf(a4), 'error');
}
});

test('toPassableError, toThrowable', t => {
Expand Down
2 changes: 2 additions & 0 deletions packages/ses/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ User-visible changes in `ses`:

# Next release

- TODO explain `SuppressedError` support

- On platforms without
[`Array.prototype.transfer`](https://github.com/tc39/proposal-resizablearraybuffer)
but with a global `structuredClone`, the ses-shim's `lockdown` will now
Expand Down
1 change: 1 addition & 0 deletions packages/ses/src/commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const {
SyntaxError,
TypeError,
AggregateError,
SuppressedError,
} = globalThis;

export const {
Expand Down
6 changes: 6 additions & 0 deletions packages/ses/src/enablements.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ export const moderateEnablements = {
name: true, // set by "node 14"?
},

// https://github.com/tc39/proposal-explicit-resource-management
'%SuppressedErrorPrototype%': {
message: true, // to match TypeErrorPrototype.message
name: true, // set by some Node?
},

'%PromisePrototype%': {
constructor: true, // set by "core-js"
},
Expand Down
42 changes: 27 additions & 15 deletions packages/ses/src/error/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
weakmapHas,
weakmapSet,
AggregateError,
SuppressedError,
getOwnPropertyDescriptors,
ownKeys,
create,
Expand Down Expand Up @@ -268,12 +269,13 @@ const tagError = (err, optErrorName = err.name) => {
* such as `stack` on v8 (Chrome, Brave, Edge?)
* - `sanitizeError` will freeze the error, preventing any correct engine from
* adding or
* altering any of the error's own properties `sanitizeError` is done.
* altering any of the error's own properties after `sanitizeError` is done.
*
* However, `sanitizeError` will not, for example, `harden`
* (i.e., deeply freeze)
* or ensure that the `cause` or `errors` property satisfy the `Passable`
* constraints. The purpose of `sanitizeError` is only to protect against
* or ensure that the `cause`, `errors`, `error`, or `suppressed` properties
* satisfy the `Passable` constraints.
* The purpose of `sanitizeError` is only to protect against
* mischief the host may have already added to the error as created,
* not to ensure that the error is actually Passable. For that,
* see `toPassableError` in `@endo/pass-style`.
Expand All @@ -285,8 +287,10 @@ export const sanitizeError = error => {
const {
name: _nameDesc,
message: _messageDesc,
errors: _errorsDesc = undefined,
cause: _causeDesc = undefined,
errors: _errorsDesc = undefined,
error: _errorDesc = undefined,
suppressed: _suppressedDesc = undefined,
stack: _stackDesc = undefined,
...restDescs
} = descs;
Expand Down Expand Up @@ -316,6 +320,8 @@ export const sanitizeError = error => {
};

/**
* TODO rewrite to be more general
*
* @type {AssertionUtilities['error']}
*/
const makeError = (
Expand All @@ -325,6 +331,8 @@ const makeError = (
errorName = undefined,
cause = undefined,
errors = undefined,
error = undefined,
suppressed = undefined,
sanitize = true,
} = {},
) => {
Expand All @@ -339,37 +347,41 @@ const makeError = (
}
const messageString = getMessageString(hiddenDetails);
const opts = cause && { cause };
let error;
let err;
if (
typeof AggregateError !== 'undefined' &&
errConstructor === AggregateError
) {
error = AggregateError(errors || [], messageString, opts);
err = AggregateError(errors || [], messageString, opts);
} else if (
typeof SuppressedError !== 'undefined' &&
errConstructor === SuppressedError
) {
err = SuppressedError(error, suppressed, messageString);
// TODO what about errors, cause?
} else {
error = /** @type {ErrorConstructor} */ (errConstructor)(
messageString,
opts,
);
err = /** @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', {
defineProperty(err, 'errors', {
value: errors,
writable: true,
enumerable: false,
configurable: true,
});
}
// TODO similarly tolerate error,suppressed ?
}
weakmapSet(hiddenMessageLogArgs, error, getLogArgs(hiddenDetails));
weakmapSet(hiddenMessageLogArgs, err, getLogArgs(hiddenDetails));
if (errorName !== undefined) {
tagError(error, errorName);
tagError(err, errorName);
}
if (sanitize) {
sanitizeError(error);
sanitizeError(err);
}
// The next line is a particularly fruitful place to put a breakpoint.
return error;
return err;
};
freeze(makeError);

Expand Down
3 changes: 3 additions & 0 deletions packages/ses/src/error/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ const ErrorInfo = {
MESSAGE: 'ERROR_MESSAGE:',
CAUSE: 'cause:',
ERRORS: 'errors:',
ERROR: 'error:',
SUPPRESSED: 'suppressed:',
};
freeze(ErrorInfo);

Expand Down Expand Up @@ -354,6 +356,7 @@ export const makeCausalConsole = (baseConsole, loggedErrorHandler) => {
// @ts-expect-error AggregateError has an `errors` property.
logErrorInfo(severity, error, ErrorInfo.ERRORS, error.errors, subErrors);
}
// TODO SuppressedError
for (const noteLogArgs of noteLogArgsArray) {
logErrorInfo(severity, error, ErrorInfo.NOTE, noteLogArgs, subErrors);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/ses/src/error/internal-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@
* MESSAGE: 'ERROR_MESSAGE:',
* CAUSE: 'cause:',
* ERRORS: 'errors:',
* ERROR: 'error:',
* SUPPRESSED: 'suppressed:',
* }} ErrorInfo
*/

Expand Down
Loading
Loading