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(marshal): tolerate extra error props #2052

Merged
merged 2 commits into from
Feb 13, 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
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

- 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.

# v0.8.1 (2022-12-23)

- Remote objects now reflect methods present on their prototype chain.
Expand Down
1 change: 1 addition & 0 deletions packages/marshal/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
},
"homepage": "https://github.com/endojs/endo#readme",
"dependencies": {
"@endo/common": "^1.0.2",
"@endo/errors": "^1.0.2",
"@endo/eventual-send": "^1.1.0",
"@endo/nat": "^5.0.2",
Expand Down
4 changes: 4 additions & 0 deletions packages/marshal/src/marshal-justin.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,10 @@ const decodeToJustin = (encoding, shouldIndent = false, slots = []) => {

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

Expand Down
19 changes: 14 additions & 5 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@endo/pass-style';

import { X, Fail, q, makeError, annotateError } from '@endo/errors';
import { objectMap } from '@endo/common/object-map.js';
import {
QCLASS,
makeEncodeToCapData,
Expand Down Expand Up @@ -260,11 +261,11 @@ export const makeMarshal = (
*/
const decodeErrorCommon = (errData, decodeRecur) => {
const { errorId = undefined, message, name, ...rest } = errData;
ownKeys(rest).length === 0 ||
Fail`unexpected encoded error properties ${q(ownKeys(rest))}`;
// TODO Must decode `cause` and `errors` properties
// capData does not transform strings. The calls to `decodeRecur`
// are for reuse by other encodings that do, such as smallcaps.
// 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);
const dErrorId = errorId && decodeRecur(errorId);
Expand All @@ -279,6 +280,14 @@ export const makeMarshal = (
? `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}`);
Comment on lines +284 to +289
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for this commentary.

}
return harden(error);
};

Expand Down
43 changes: 42 additions & 1 deletion packages/marshal/test/test-marshal-capdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ import { passStyleOf, Far } from '@endo/pass-style';
import { makeMarshal } from '../src/marshal.js';
import { roundTripPairs } from './marshal-test-data.js';

const { freeze, isFrozen, create, prototype: objectPrototype } = Object;
const {
freeze,
isFrozen,
create,
prototype: objectPrototype,
getPrototypeOf,
} = Object;

const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ (
// eslint-disable-next-line no-undef
Expand Down Expand Up @@ -147,6 +153,41 @@ test('unserialize errors', t => {
t.is(em3.message, 'msg3');
});

test('unserialize extended errors', t => {
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.false('extraProp' in aggErr);
t.false('cause' in aggErr);
t.false('errors' in aggErr);
console.log('error with extra prop', aggErr);

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

test('passStyleOf null is "null"', t => {
t.assert(passStyleOf(null), 'null');
});
Expand Down
43 changes: 42 additions & 1 deletion packages/marshal/test/test-marshal-smallcaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import { makeMarshal } from '../src/marshal.js';

import { roundTripPairs } from './marshal-test-data.js';

const { freeze, isFrozen, create, prototype: objectPrototype } = Object;
const {
freeze,
isFrozen,
create,
prototype: objectPrototype,
getPrototypeOf,
} = Object;

const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ (
// eslint-disable-next-line no-undef
Expand Down Expand Up @@ -153,6 +159,41 @@ test('smallcaps unserialize errors', t => {
t.is(em3.message, 'msg3');
});

test('smallcaps unserialize extended errors', t => {
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(
'#{"#error":"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(
'#{"#error":"msg","name":"AggregateError","extraProp":"foo","cause":"bar","errors":["zip","zap"]}',
);
t.is(getPrototypeOf(aggErr), Error.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);

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

test('smallcaps mal-formed @qclass', t => {
const { unserialize } = makeTestMarshal();
const uns = body => unserialize({ body, slots: [] });
Expand Down
Loading