Skip to content

Commit

Permalink
fix(ses,pass-style,marshal): fix 2428, generalize passable errors, th…
Browse files Browse the repository at this point in the history
…rowables
  • Loading branch information
erights committed Nov 17, 2024
1 parent b3f0c56 commit c11a861
Show file tree
Hide file tree
Showing 25 changed files with 379 additions and 147 deletions.
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
10 changes: 9 additions & 1 deletion packages/eslint-plugin/lib/configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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',
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.6.0 (2024-10-22)

- `compareRank` now short-circuits upon encountering remotables to compare,
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
151 changes: 84 additions & 67 deletions packages/pass-style/src/error.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
/// <reference types="ses"/>

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.
Expand All @@ -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],
]),
);

Expand All @@ -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.
Expand All @@ -61,7 +70,6 @@ const checkErrorLike = (candidate, check = undefined) => {
);
};
harden(checkErrorLike);
/// <reference types="ses"/>

/**
* Validating error objects are passable raises a tension between security
Expand All @@ -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 &&
Expand All @@ -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<string,any>} */ (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({
Expand All @@ -209,5 +225,6 @@ export const ErrorHelper = harden({
canBeValid: checkErrorLike,

assertValid: (candidate, passStyleOfRecur) =>
checkRecursivelyPassableError(candidate, passStyleOfRecur, assertChecker),
checkErrorLike(candidate, assertChecker) &&
checkRecursivelyThrowable(candidate, passStyleOfRecur, assertChecker),
});
Loading

0 comments on commit c11a861

Please sign in to comment.