From c11a86123fbbfe3fbcd41464e356e3b347a6b03f Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Mon, 26 Aug 2024 19:03:57 -0700 Subject: [PATCH] fix(ses,pass-style,marshal): fix 2428, generalize passable errors, throwables --- packages/common/NEWS.md | 4 + packages/errors/NEWS.md | 5 + .../eslint-plugin/lib/configs/recommended.js | 10 +- packages/marshal/NEWS.md | 4 + packages/marshal/src/marshal-justin.js | 2 + packages/marshal/src/marshal.js | 16 +- packages/marshal/src/types.js | 2 + packages/marshal/test/marshal-capdata.test.js | 2 + .../marshal/test/marshal-smallcaps.test.js | 2 + packages/pass-style/NEWS.md | 4 + packages/pass-style/src/error.js | 151 ++++++++++-------- packages/pass-style/src/passStyleOf.js | 29 +++- packages/pass-style/test/errors.test.js | 7 +- packages/ses/NEWS.md | 2 + packages/ses/src/commons.js | 1 + packages/ses/src/enablements.js | 6 + packages/ses/src/error/assert.js | 151 +++++++++++------- packages/ses/src/error/console.js | 3 + packages/ses/src/error/internal-types.js | 2 + packages/ses/src/permits.js | 70 +++++++- .../aggregate-error-console-demo.test.js | 2 + .../error/aggregate-error-console.test.js | 2 + .../ses/test/error/aggregate-error.test.js | 2 + packages/ses/test/error/error-cause.test.js | 2 + packages/ses/types.d.ts | 45 +++++- 25 files changed, 379 insertions(+), 147 deletions(-) diff --git a/packages/common/NEWS.md b/packages/common/NEWS.md index fd9f706c3c..aac7ee8c7b 100644 --- a/packages/common/NEWS.md +++ b/packages/common/NEWS.md @@ -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 diff --git a/packages/errors/NEWS.md b/packages/errors/NEWS.md index 99c636c9da..6a1c99e21a 100644 --- a/packages/errors/NEWS.md +++ b/packages/errors/NEWS.md @@ -1,5 +1,10 @@ User-visible changes in `@endo/errors`: +# Next release + +- TODO explain `SuppressedError` support + + # v1.1.0 (2024-02-22) - `AggegateError` support diff --git a/packages/eslint-plugin/lib/configs/recommended.js b/packages/eslint-plugin/lib/configs/recommended.js index 6e0da94c8a..02db82c945 100644 --- a/packages/eslint-plugin/lib/configs/recommended.js +++ b/packages/eslint-plugin/lib/configs/recommended.js @@ -7,6 +7,10 @@ module.exports = { node: false, commonjs: false, }, + // Allow what the SES-shim makes powerless. Co-maintain with + // `universalPropertyNames` from `ses/src/permits.js`. + // TODO align better with `universalPropertyNames` to make + // co-maintenance easier. globals: { assert: 'readonly', console: 'readonly', @@ -17,7 +21,6 @@ module.exports = { URL: 'readonly', URLSearchParams: 'readonly', - // Allow what SES makes powerless, copied from its whitelist // *** Constructor Properties of the Global Object Array: 'readonly', ArrayBuffer: 'readonly', @@ -63,6 +66,11 @@ module.exports = { HandledPromise: 'readonly', // https://github.com/endojs/endo/issues/550 AggregateError: 'readonly', + // https://github.com/tc39/proposal-explicit-resource-management + AsyncDisposableStack: 'readonly', + DisposableStack: 'readonly', + SuppressedError: 'readonly', + }, rules: { '@endo/assert-fail-as-throw': 'error', diff --git a/packages/marshal/NEWS.md b/packages/marshal/NEWS.md index 7731cd163c..16c0327a8a 100644 --- a/packages/marshal/NEWS.md +++ b/packages/marshal/NEWS.md @@ -1,5 +1,9 @@ User-visible changes in `@endo/marshal`: +# Next release + +TODO explain `SuppressedError` support + # v1.6.0 (2024-10-22) - `compareRank` now short-circuits upon encountering remotables to compare, diff --git a/packages/marshal/src/marshal-justin.js b/packages/marshal/src/marshal-justin.js index 681c14e962..0a6c3da8ab 100644 --- a/packages/marshal/src/marshal-justin.js +++ b/packages/marshal/src/marshal-justin.js @@ -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)})`); } diff --git a/packages/marshal/src/marshal.js b/packages/marshal/src/marshal.js index 86ad6891ac..fe67a69f45 100644 --- a/packages/marshal/src/marshal.js +++ b/packages/marshal/src/marshal.js @@ -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') { @@ -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} @@ -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 @@ -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, diff --git a/packages/marshal/src/types.js b/packages/marshal/src/types.js index 8f73c6c936..58bbb7a0f3 100644 --- a/packages/marshal/src/types.js +++ b/packages/marshal/src/types.js @@ -38,6 +38,8 @@ export {}; * errorId?: string, * cause?: Encoding, * errors?: Encoding[], + * error?: Encoding, + * suppressed?: Encoding, * } | * EncodingClass<'slot'> & { index: number, * iface?: string diff --git a/packages/marshal/test/marshal-capdata.test.js b/packages/marshal/test/marshal-capdata.test.js index 90e7d10819..20dfaa7916 100644 --- a/packages/marshal/test/marshal-capdata.test.js +++ b/packages/marshal/test/marshal-capdata.test.js @@ -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'); }); diff --git a/packages/marshal/test/marshal-smallcaps.test.js b/packages/marshal/test/marshal-smallcaps.test.js index 87ed067ba3..4a778e7bb4 100644 --- a/packages/marshal/test/marshal-smallcaps.test.js +++ b/packages/marshal/test/marshal-smallcaps.test.js @@ -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: [] }); diff --git a/packages/pass-style/NEWS.md b/packages/pass-style/NEWS.md index cea0c9aae4..7ff9590bdd 100644 --- a/packages/pass-style/NEWS.md +++ b/packages/pass-style/NEWS.md @@ -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. diff --git a/packages/pass-style/src/error.js b/packages/pass-style/src/error.js index 83fecc90ac..3855249a8f 100644 --- a/packages/pass-style/src/error.js +++ b/packages/pass-style/src/error.js @@ -1,12 +1,13 @@ /// import { q } from '@endo/errors'; -import { assertChecker, CX } from './passStyle-helpers.js'; +import { assertChecker, isObject, CX } from './passStyle-helpers.js'; /** @import {PassStyleHelper} from './internal-types.js' */ -/** @import {Checker, PassStyle, PassStyleOf} from './types.js' */ +/** @import {Checker, PassStyle, CopyTagged, Passable} from './types.js' */ -const { getPrototypeOf, getOwnPropertyDescriptors, hasOwn, entries } = Object; +const { getPrototypeOf, getOwnPropertyDescriptors, hasOwn, entries, values } = + Object; // TODO: Maintenance hazard: Coordinate with the list of errors in the SES // whilelist. @@ -27,6 +28,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], ]), ); @@ -35,9 +38,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. @@ -61,7 +70,6 @@ const checkErrorLike = (candidate, check = undefined) => { ); }; harden(checkErrorLike); -/// /** * Validating error objects are passable raises a tension between security @@ -85,26 +93,21 @@ export const isErrorLike = candidate => checkErrorLike(candidate); harden(isErrorLike); /** + * An own property of a passable error must be a data property whose value is + * a throwable value. + * * @param {string} propName * @param {PropertyDescriptor} desc * @param {(val: any) => PassStyle} passStyleOfRecur * @param {Checker} [check] * @returns {boolean} */ -export const checkRecursivelyPassableErrorPropertyDesc = ( +export const checkRecursivelyPassableErrorOwnPropertyDesc = ( propName, desc, passStyleOfRecur, check = undefined, ) => { - if (desc.enumerable) { - return ( - !!check && - CX(check)`Passable Error ${q( - propName, - )} own property must not be enumerable: ${desc}` - ); - } if (!hasOwn(desc, 'value')) { return ( !!check && @@ -125,82 +128,95 @@ export const checkRecursivelyPassableErrorPropertyDesc = ( )} own property must be a string: ${value}`) ); } - case 'cause': { - // eslint-disable-next-line no-use-before-define - return checkRecursivelyPassableError(value, passStyleOfRecur, check); - } - case 'errors': { - if (!Array.isArray(value) || passStyleOfRecur(value) !== 'copyArray') { - return ( - !!check && - CX(check)`Passable Error ${q( - propName, - )} own property must be a copyArray: ${value}` - ); - } - return value.every(err => - // eslint-disable-next-line no-use-before-define - checkRecursivelyPassableError(err, passStyleOfRecur, check), - ); - } default: { break; } } - return ( - !!check && - CX(check)`Passable Error has extra unpassed property ${q(propName)}` - ); + // eslint-disable-next-line no-use-before-define + return checkRecursivelyThrowable(value, passStyleOfRecur, check); }; -harden(checkRecursivelyPassableErrorPropertyDesc); +harden(checkRecursivelyPassableErrorOwnPropertyDesc); /** + * `candidate` is throwable if it contains only data and passable errors. + * * @param {unknown} candidate * @param {(val: any) => PassStyle} passStyleOfRecur * @param {Checker} [check] * @returns {boolean} */ -export const checkRecursivelyPassableError = ( +export const checkRecursivelyThrowable = ( candidate, passStyleOfRecur, check = undefined, ) => { - if (!checkErrorLike(candidate, check)) { - return false; - } - const proto = getPrototypeOf(candidate); - const { name } = proto; - const errConstructor = getErrorConstructor(name); - if (errConstructor === undefined || errConstructor.prototype !== proto) { - return ( - !!check && - CX( + if (checkErrorLike(candidate, undefined)) { + const proto = getPrototypeOf(candidate); + const { name } = proto; + const errConstructor = getErrorConstructor(name); + if (errConstructor === undefined || errConstructor.prototype !== proto) { + return ( + !!check && + CX( + check, + )`Passable Error must inherit from an error class .prototype: ${candidate}` + ); + } + const descs = getOwnPropertyDescriptors(candidate); + if (!('message' in descs)) { + return ( + !!check && + CX( + check, + )`Passable Error must have an own "message" string property: ${candidate}` + ); + } + + return entries(descs).every(([propName, desc]) => + checkRecursivelyPassableErrorOwnPropertyDesc( + propName, + desc, + passStyleOfRecur, check, - )`Passable Error must inherit from an error class .prototype: ${candidate}` + ), ); } - const descs = getOwnPropertyDescriptors(candidate); - if (!('message' in descs)) { - return ( - !!check && - CX( + const passStyle = passStyleOfRecur(candidate); + if (!isObject(candidate)) { + // All passable primitives are throwable + return true; + } + switch (passStyle) { + case 'copyArray': { + return /** @type {Passable[]} */ (candidate).every(element => + checkRecursivelyThrowable(element, passStyleOfRecur, check), + ); + } + case 'copyRecord': { + return values(/** @type {Record} */ (candidate)).every( + value => checkRecursivelyThrowable(value, passStyleOfRecur, check), + ); + } + case 'tagged': { + return checkRecursivelyThrowable( + /** @type {CopyTagged} */ (candidate).payload, + passStyleOfRecur, check, - )`Passable Error must have an own "message" string property: ${candidate}` - ); + ); + } + default: { + return ( + !!check && + CX(check)`A throwable cannot contain a ${q(passStyle)}: ${candidate}` + ); + } } - - return entries(descs).every(([propName, desc]) => - checkRecursivelyPassableErrorPropertyDesc( - propName, - desc, - passStyleOfRecur, - check, - ), - ); }; -harden(checkRecursivelyPassableError); +harden(checkRecursivelyThrowable); /** + * A passable error is a throwable error and contains only throwable values. + * * @type {PassStyleHelper} */ export const ErrorHelper = harden({ @@ -209,5 +225,6 @@ export const ErrorHelper = harden({ canBeValid: checkErrorLike, assertValid: (candidate, passStyleOfRecur) => - checkRecursivelyPassableError(candidate, passStyleOfRecur, assertChecker), + checkErrorLike(candidate, assertChecker) && + checkRecursivelyThrowable(candidate, passStyleOfRecur, assertChecker), }); diff --git a/packages/pass-style/src/passStyleOf.js b/packages/pass-style/src/passStyleOf.js index 7c4dd78b2e..5b6b6ec21c 100644 --- a/packages/pass-style/src/passStyleOf.js +++ b/packages/pass-style/src/passStyleOf.js @@ -11,8 +11,8 @@ import { CopyRecordHelper } from './copyRecord.js'; import { TaggedHelper } from './tagged.js'; import { ErrorHelper, - checkRecursivelyPassableErrorPropertyDesc, - checkRecursivelyPassableError, + checkRecursivelyPassableErrorOwnPropertyDesc, + checkRecursivelyThrowable, getErrorConstructor, isErrorLike, } from './error.js'; @@ -260,7 +260,7 @@ harden(isPassable); * @returns {boolean} */ const isPassableErrorPropertyDesc = (name, desc) => - checkRecursivelyPassableErrorPropertyDesc(name, desc, passStyleOf); + checkRecursivelyPassableErrorOwnPropertyDesc(name, desc, passStyleOf); /** * After hardening, if `err` is a passable error, return it. @@ -277,26 +277,43 @@ const isPassableErrorPropertyDesc = (name, desc) => */ export const toPassableError = err => { harden(err); - if (checkRecursivelyPassableError(err, passStyleOf)) { + if (checkRecursivelyThrowable(err, passStyleOf)) { 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); diff --git a/packages/pass-style/test/errors.test.js b/packages/pass-style/test/errors.test.js index 2c0fd0fcce..98216c0e1e 100644 --- a/packages/pass-style/test/errors.test.js +++ b/packages/pass-style/test/errors.test.js @@ -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 => { @@ -71,7 +76,7 @@ test('toPassableError, toThrowable', t => { // a throwable singleton copyArray containing a toThrowable(e), i.e., // an error like e2. t.throws(() => toThrowable(notYetCoercable), { - message: 'Passable Error has extra unpassed property "foo"', + message: 'A throwable cannot contain a "remotable": "[Alleged: Foo]"', }); const throwable = harden([e2, { e2 }, makeTagged('e2', e2)]); diff --git a/packages/ses/NEWS.md b/packages/ses/NEWS.md index 4bb7cba98e..6eee9c3d24 100644 --- a/packages/ses/NEWS.md +++ b/packages/ses/NEWS.md @@ -25,6 +25,8 @@ User-visible changes in `ses`: # v1.9.0 (2024-10-10) +- 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 diff --git a/packages/ses/src/commons.js b/packages/ses/src/commons.js index b4f8f6f391..1ce95b9dcf 100644 --- a/packages/ses/src/commons.js +++ b/packages/ses/src/commons.js @@ -50,6 +50,7 @@ export const { SyntaxError, TypeError, AggregateError, + SuppressedError, } = globalThis; export const { diff --git a/packages/ses/src/enablements.js b/packages/ses/src/enablements.js index bb352b3cb5..79a0a5cae7 100644 --- a/packages/ses/src/enablements.js +++ b/packages/ses/src/enablements.js @@ -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" }, diff --git a/packages/ses/src/error/assert.js b/packages/ses/src/error/assert.js index f4a645c5b0..c4dfc53ef6 100644 --- a/packages/ses/src/error/assert.js +++ b/packages/ses/src/error/assert.js @@ -35,10 +35,9 @@ import { weakmapHas, weakmapSet, AggregateError, + SuppressedError, getOwnPropertyDescriptors, ownKeys, - create, - objectPrototype, objectHasOwnProperty, } from '../commons.js'; import { an, bestEffortStringify } from './stringify-utils.js'; @@ -254,6 +253,28 @@ const tagError = (err, optErrorName = err.name) => { return errorTag; }; +/** + * The own properties that we expect to see on an error object when it is + * first constructed. Any other properties added by the constructor are + * beyond what is standard. When sanitizing, these should be removed + * immediately after construction. + * Currently known examples: + * - `fileName` and `lineNumber` on Firefox/SpiderMonkey + * - `line` on Safari/JSC + * + * These example properties in particular carry information that should + * normally be redacted. + */ +const expectedErrorOwnProps = freeze({ + __proto__: null, // this is syntax, not a property named "__proto__" + message: true, + stack: true, + cause: true, + errors: true, + error: true, + suppressed: true, +}); + /** * Make reasonable best efforts to make a `Passable` error. * - `sanitizeError` will remove any "extraneous" own properties already added @@ -267,53 +288,47 @@ const tagError = (err, optErrorName = err.name) => { * added by the host are data * properties, converting accessor properties to data properties as needed, * 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. * * 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`. * - * @param {Error} error + * @param {Error} err */ -export const sanitizeError = error => { - const descs = getOwnPropertyDescriptors(error); - const { - name: _nameDesc, - message: _messageDesc, - errors: _errorsDesc = undefined, - cause: _causeDesc = undefined, - stack: _stackDesc = undefined, - ...restDescs - } = descs; - - const restNames = ownKeys(restDescs); - if (restNames.length >= 1) { - for (const name of restNames) { - delete error[name]; +export const sanitizeError = err => { + const descs = getOwnPropertyDescriptors(err); + let needNote = false; + const droppedNote = {}; + + for (const name of ownKeys(err)) { + if (expectedErrorOwnProps[name]) { + // @ts-expect-error TS still confused by symbols as property names + const desc = descs[name]; + if (desc && objectHasOwnProperty(desc, 'get')) { + defineProperty(err, name, { + value: err[name], // invoke the getter to convert to data property + }); + } + } else { + needNote = true; + defineProperty(droppedNote, name, { + value: err[name], // invoke the getter to convert to data property + }); + delete err[name]; } - const droppedNote = create(objectPrototype, restDescs); + } + if (needNote) { // eslint-disable-next-line no-use-before-define note( - error, + err, redactedDetails`originally with properties ${quote(droppedNote)}`, ); } - for (const name of ownKeys(error)) { - // @ts-expect-error TS still confused by symbols as property names - const desc = descs[name]; - if (desc && objectHasOwnProperty(desc, 'get')) { - defineProperty(error, name, { - value: error[name], // invoke the getter to convert to data property - }); - } - } - freeze(error); }; /** @@ -324,9 +339,12 @@ const makeError = ( errConstructor = globalThis.Error, { errorName = undefined, - cause = undefined, - errors = undefined, sanitize = true, + options = undefined, + properties = undefined, + + cause = undefined, // Deprecated. Should be provided in `properties` + errors = undefined, // Deprecated. Should be provided in `properties` } = {}, ) => { if (typeof optDetails === 'string') { @@ -338,39 +356,60 @@ const makeError = ( if (hiddenDetails === undefined) { throw TypeError(`unrecognized details ${quote(optDetails)}`); } + // The messageString is overridden by `message` if provided. const messageString = getMessageString(hiddenDetails); - const opts = cause && { cause }; - let error; + + let err; if ( typeof AggregateError !== 'undefined' && errConstructor === AggregateError ) { - error = AggregateError(errors || [], messageString, opts); + // First arg overridden by `errors` is provided. + // A `cause` in `options` is overridden by `cause` if provided. + err = AggregateError([], messageString, options); + } else if ( + typeof SuppressedError !== 'undefined' && + errConstructor === SuppressedError + ) { + // First two args overridden by `error` and `suppressed` if provided. + // Bizarrely, `SuppressedError` has no options argument and therefore + // no direct way to endow it with a `cause`. Nevertheless, + // it will be given a `cause` if provided. + err = SuppressedError(undefined, undefined, messageString); } else { - error = /** @type {ErrorConstructor} */ (errConstructor)( + // A `cause` in `options` is overridden by `cause` if provided. + err = /** @type {ErrorConstructor} */ (errConstructor)( messageString, - opts, + options, ); - 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 (sanitize) { + sanitizeError(err); + } + + weakmapSet(hiddenMessageLogArgs, err, getLogArgs(hiddenDetails)); if (errorName !== undefined) { - tagError(error, errorName); + tagError(err, errorName); + } + + // TODO This silently drops non-enumerable properties. Do we care? + const props = { ...properties }; + if (cause !== undefined) { + props.cause = cause; + } + if (errors !== undefined) { + props.errors = errors; + } + for (const name of ownKeys(props)) { + defineProperty(err, name, { + value: props[name], + }); } if (sanitize) { - sanitizeError(error); + freeze(err); } // The next line is a particularly fruitful place to put a breakpoint. - return error; + return err; }; freeze(makeError); diff --git a/packages/ses/src/error/console.js b/packages/ses/src/error/console.js index 5151a609a3..bf903799da 100644 --- a/packages/ses/src/error/console.js +++ b/packages/ses/src/error/console.js @@ -210,6 +210,8 @@ const ErrorInfo = { MESSAGE: 'ERROR_MESSAGE:', CAUSE: 'cause:', ERRORS: 'errors:', + ERROR: 'error:', + SUPPRESSED: 'suppressed:', }; freeze(ErrorInfo); @@ -359,6 +361,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); } diff --git a/packages/ses/src/error/internal-types.js b/packages/ses/src/error/internal-types.js index 214ff08ab3..d0fb3c34b2 100644 --- a/packages/ses/src/error/internal-types.js +++ b/packages/ses/src/error/internal-types.js @@ -78,6 +78,8 @@ * MESSAGE: 'ERROR_MESSAGE:', * CAUSE: 'cause:', * ERRORS: 'errors:', + * ERROR: 'error:', + * SUPPRESSED: 'suppressed:', * }} ErrorInfo */ diff --git a/packages/ses/src/permits.js b/packages/ses/src/permits.js index c5745903ee..1e89b8cd18 100644 --- a/packages/ses/src/permits.js +++ b/packages/ses/src/permits.js @@ -33,6 +33,10 @@ export const constantProperties = { * Properties of all global objects. * Must be powerless. * Maps from property name to the intrinsic name in the permits. + * Co-maintain with `module.exports.globals` of + * `eslint-plugin/lib/configs/recommended.js`. + * TODO align better with `globals` to make + * co-maintenance easier. */ export const universalPropertyNames = { // *** Function Properties of the Global Object @@ -89,6 +93,11 @@ export const universalPropertyNames = { // https://github.com/endojs/endo/issues/550 AggregateError: 'AggregateError', + // https://github.com/tc39/proposal-explicit-resource-management + AsyncDisposableStack: 'AsyncDisposableStack', + DisposableStack: 'DisposableStack', + SuppressedError: 'SuppressedError', + // *** Other Properties of the Global Object JSON: 'JSON', @@ -207,6 +216,8 @@ const NativeErrors = [ // Commented out to accommodate platforms prior to AggregateError. // Instead, conditional push below. // AggregateError, + // Likewise https://github.com/tc39/proposal-explicit-resource-management + // SuppressedError, ]; if (typeof AggregateError !== 'undefined') { @@ -214,6 +225,11 @@ if (typeof AggregateError !== 'undefined') { arrayPush(NativeErrors, AggregateError); } +if (typeof SuppressedError !== 'undefined') { + // Conditional, to accommodate platforms prior to SuppressedError + arrayPush(NativeErrors, SuppressedError); +} + export { NativeErrors }; /** @@ -537,8 +553,10 @@ export const permitted = { '%SharedSymbol%': { // Properties of the Symbol Constructor '[[Proto]]': '%FunctionPrototype%', + // https://github.com/tc39/proposal-explicit-resource-management asyncDispose: 'symbol', asyncIterator: 'symbol', + // https://github.com/tc39/proposal-explicit-resource-management dispose: 'symbol', for: fn, hasInstance: 'symbol', @@ -608,7 +626,7 @@ export const permitted = { // Seen on FF and XS stack: accessor, // Superfluously present in some versions of V8. - // https://github.com/tc39/notes/blob/master/meetings/2021-10/oct-26.md#:~:text=However%2C%20Chrome%2093,and%20node%2016.11. + // https://github.com/tc39/notes/blob/main/meetings/2021-10/oct-26.md#tightening-host-restrictions-to-improve-testing cause: false, }, @@ -622,6 +640,8 @@ export const permitted = { URIError: NativeError('%URIErrorPrototype%'), // https://github.com/endojs/endo/issues/550 AggregateError: NativeError('%AggregateErrorPrototype%'), + // https://github.com/tc39/proposal-explicit-resource-management + SuppressedError: NativeError('%SuppressedErrorPrototype%'), '%EvalErrorPrototype%': NativeErrorPrototype('EvalError'), '%RangeErrorPrototype%': NativeErrorPrototype('RangeError'), @@ -631,6 +651,8 @@ export const permitted = { '%URIErrorPrototype%': NativeErrorPrototype('URIError'), // https://github.com/endojs/endo/issues/550 '%AggregateErrorPrototype%': NativeErrorPrototype('AggregateError'), + // https://github.com/tc39/proposal-explicit-resource-management + '%SuppressedErrorPrototype%': NativeErrorPrototype('SuppressedError'), // *** Numbers and Dates @@ -1293,6 +1315,44 @@ export const permitted = { SharedArrayBuffer: false, // UNSAFE and purposely suppressed. '%SharedArrayBufferPrototype%': false, // UNSAFE and purposely suppressed. + // https://github.com/tc39/proposal-explicit-resource-management + DisposableStack: { + '[[Proto]]': '%FunctionPrototype%', + prototype: '%DisposableStackPrototype%', + }, + + // https://github.com/tc39/proposal-explicit-resource-management + '%DisposableStackPrototype%': { + constructor: 'DisposableStack', + adopt: fn, + defer: fn, + dispose: fn, + disposed: getter, + move: fn, + use: fn, + '@@dispose': fn, + '@@toStringTag': 'string', + }, + + // https://github.com/tc39/proposal-explicit-resource-management + AsyncDisposableStack: { + '[[Proto]]': '%FunctionPrototype%', + prototype: '%AsyncDisposableStackPrototype%', + }, + + // https://github.com/tc39/proposal-explicit-resource-management + '%AsyncDisposableStackPrototype%': { + constructor: 'AsyncDisposableStack', + adopt: fn, + defer: fn, + disposeAsync: fn, + disposed: getter, + move: fn, + use: fn, + '@@asyncDispose': fn, + '@@toStringTag': 'string', + }, + DataView: { // Properties of the DataView Constructor '[[Proto]]': '%FunctionPrototype%', @@ -1373,8 +1433,8 @@ export const permitted = { '@@toStringTag': 'string', // https://github.com/tc39/proposal-async-iterator-helpers toAsync: fn, - // See https://github.com/Moddable-OpenSource/moddable/issues/523#issuecomment-1942904505 - '@@dispose': false, + // https://github.com/tc39/proposal-explicit-resource-management + '@@dispose': fn, }, // https://github.com/tc39/proposal-iterator-helpers @@ -1417,8 +1477,8 @@ export const permitted = { every: fn, find: fn, '@@toStringTag': 'string', - // See https://github.com/Moddable-OpenSource/moddable/issues/523#issuecomment-1942904505 - '@@asyncDispose': false, + // https://github.com/tc39/proposal-explicit-resource-management + '@@asyncDispose': fn, }, // https://github.com/tc39/proposal-async-iterator-helpers diff --git a/packages/ses/test/error/aggregate-error-console-demo.test.js b/packages/ses/test/error/aggregate-error-console-demo.test.js index ba2da6ae16..05a3dd4468 100644 --- a/packages/ses/test/error/aggregate-error-console-demo.test.js +++ b/packages/ses/test/error/aggregate-error-console-demo.test.js @@ -22,3 +22,5 @@ test('aggregate error console demo', t => { console.log('log1', a1); t.is(a1.cause, e2); }); + +// TODO SuppressedError diff --git a/packages/ses/test/error/aggregate-error-console.test.js b/packages/ses/test/error/aggregate-error-console.test.js index e9981c4fe9..4d44be44ec 100644 --- a/packages/ses/test/error/aggregate-error-console.test.js +++ b/packages/ses/test/error/aggregate-error-console.test.js @@ -46,3 +46,5 @@ test('aggregate error console', t => { { wrapWithCausal: true }, ); }); + +// TODO SuppressedError diff --git a/packages/ses/test/error/aggregate-error.test.js b/packages/ses/test/error/aggregate-error.test.js index 82f397799c..fd02cba3c9 100644 --- a/packages/ses/test/error/aggregate-error.test.js +++ b/packages/ses/test/error/aggregate-error.test.js @@ -53,3 +53,5 @@ test('Promise.any aggregate error', async t => { }); } }); + +// TODO SuppressedError diff --git a/packages/ses/test/error/error-cause.test.js b/packages/ses/test/error/error-cause.test.js index 5cd32091f5..7952d2f035 100644 --- a/packages/ses/test/error/error-cause.test.js +++ b/packages/ses/test/error/error-cause.test.js @@ -41,3 +41,5 @@ test('error cause', t => { configurable: true, }); }); + +// TODO SuppressedError diff --git a/packages/ses/types.d.ts b/packages/ses/types.d.ts index cfde6dd017..30c9f94f33 100644 --- a/packages/ses/types.d.ts +++ b/packages/ses/types.d.ts @@ -174,10 +174,43 @@ export interface AssertMakeErrorOptions { */ errorName?: string; + /** + * Defaults to true. If true, `makeError` will apply `sanitizeError` + * to the error before returning it. See the comments on + * {@link sanitizeError}. + */ + sanitize?: boolean; + + /** + * Options to be passed as the `options` argument to the error constructor. + * Bizarrely, the `SuppressedError` constructor has no options argument. + * Thus, if the constructor is `SuppressedError` and a non-empty + * `options` argument is present, we would normally report this inconsistency + * as an error. However, we do not, because the original `SuppressedError` + * we're trying to report likely has more relevant diagnostics than an + * error that happens in the attempt to report this error. + * + * For those error constructors that do take an `options` argument, + * currently the only defined option is `cause`. If `cause` is provided + * both as `options.cause` and `properties.cause`, the latter will override + * the former. + */ + options?: ErrorOptions; + + /** + * Extra properties to be added to the error after `sanitizeError` and + * before freezing. Only the value of the properties are added, not + * the descriptor. IOW, if any property is an accessor, the getter is + * called to get the value, which is used to make a data property. + */ + properties?: object; + /** * Discloses the error that caused this one, typically from a lower * layer of abstraction. This is represented by a public `cause` data property * on the error, not a hidden annotation. + * + * @deprecated Should be provided in `properties` or `options` */ cause?: Error; @@ -187,15 +220,10 @@ export interface AssertMakeErrorOptions { * typically by `Promise.any`. But `makeError` allows it on any error. * This is represented by a public `errors` data property on the error, * not a hidden annotation. + * + * @deprecated Should be provided in `properties` */ errors?: Error[]; - - /** - * Defaults to true. If true, `makeError` will apply `sanitizeError` - * to the error before returning it. See the comments on - * {@link sanitizeError}. - */ - sanitize?: boolean; } // TODO inline overloading @@ -265,7 +293,8 @@ interface StringablePayload { */ export type GenericErrorConstructor = | ErrorConstructor - | AggregateErrorConstructor; + | AggregateErrorConstructor + | SuppressedErrorConstructor; /** * To make an `assert` which terminates some larger unit of computation