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): temp patch in Endo 2355,2359 line-number workaround #9711

Merged
merged 6 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
8 changes: 4 additions & 4 deletions packages/SwingSet/test/snapshots/xsnap-store.test.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ Generated by [AVA](https://avajs.dev).
{
compressSeconds: 0,
dbSaveSeconds: 0,
hash: 'bd10d954d29157e1ab9a068b6bdeb8bb7157489e11dc71b2961f4cab9a7d135c',
uncompressedSize: 827051,
hash: '30d2d6ce649f1f3b9d5dd48404ee32c4ff0cc9935b7aa4498c0bcd218eedca71',
uncompressedSize: 827651,
}

> after use of harden() - sensitive to SES-shim, XS, and supervisor

{
compressSeconds: 0,
dbSaveSeconds: 0,
hash: 'ba3a51da404ab14b6366df8966df683bc4f8ffd99f2dc12aabfa3b7728f39952',
uncompressedSize: 827211,
hash: '46d4988aa5eed7354684b76362fa8b6049e4467139e064a5b73fa8e80bed26a7',
uncompressedSize: 827811,
}
Binary file modified packages/SwingSet/test/snapshots/xsnap-store.test.js.snap
Binary file not shown.
42 changes: 42 additions & 0 deletions packages/boot/test/bootstrapTests/stack-linenumbers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

/*
lots of vertical space to test the display of error linenumbers
when running TypeScript tests under Ava on Node.

This is not an automated test of anything. Rather, its purpose is
visual inspection of the output.

With the `SUPPRESS_NODE_ERROR_TAMING` environment variable absent
or set to `'disabled'`, you should see a stack trace that includes
something like

```
boot/test/bootstrapTests/stack-linenumbers.test.ts:1:104
```

This is because the TypeScript compiler compiles this file into one line
of JavaScript with a sourceMap that should map back into original
positions in this file. Node specifically makes use of that sourceMap
to produce original linenumbers. However, Node does this in a way
that resists virtualization, so the normal SES error taming cannot use
this sourceMap info.

However, if you also set the `SUPPRESS_NODE_ERROR_TAMING` environment
variable `'enabled'`, for example by doing

```sh
$ export SUPPRESS_NODE_ERROR_TAMING=enabled
```
at a bash shell, then when you run this test you should instead see
something like
```
boot/test/bootstrapTests/stack-linenumbers.test.ts:40:32
```

*/

test('foo', t => {
erights marked this conversation as resolved.
Show resolved Hide resolved
t.log('look at linenumbers', Error('what me worry'));
t.pass();
});
13 changes: 13 additions & 0 deletions patches/@endo+lockdown+1.0.7.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/node_modules/@endo/lockdown/commit-debug.js b/node_modules/@endo/lockdown/commit-debug.js
index 9595aa0..03e02a2 100644
--- a/node_modules/@endo/lockdown/commit-debug.js
+++ b/node_modules/@endo/lockdown/commit-debug.js
@@ -21,7 +21,7 @@ lockdown({
// NOTE TO REVIEWERS: If you see the following line commented out,
// this may be a development accident that should be fixed before merging.
//
- errorTaming: 'unsafe',
+ errorTaming: 'unsafe-debug',

// The default `{stackFiltering: 'concise'}` setting usually makes for a
// better debugging experience, by severely reducing the noisy distractions
167 changes: 167 additions & 0 deletions patches/ses+1.5.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
diff --git a/node_modules/ses/src/error/tame-error-constructor.js b/node_modules/ses/src/error/tame-error-constructor.js
index 2788c42..db59952 100644
--- a/node_modules/ses/src/error/tame-error-constructor.js
+++ b/node_modules/ses/src/error/tame-error-constructor.js
@@ -7,6 +7,7 @@ import {
setPrototypeOf,
getOwnPropertyDescriptor,
defineProperty,
+ getOwnPropertyDescriptors,
} from '../commons.js';
import { NativeErrors } from '../permits.js';
import { tameV8ErrorConstructor } from './tame-v8-error-constructor.js';
@@ -29,12 +30,17 @@ const tamedMethods = {
return '';
},
};
+let initialGetStackString = tamedMethods.getStackString;

export default function tameErrorConstructor(
errorTaming = 'safe',
stackFiltering = 'concise',
) {
- if (errorTaming !== 'safe' && errorTaming !== 'unsafe') {
+ if (
+ errorTaming !== 'safe' &&
+ errorTaming !== 'unsafe' &&
+ errorTaming !== 'unsafe-debug'
+ ) {
throw TypeError(`unrecognized errorTaming ${errorTaming}`);
}
if (stackFiltering !== 'concise' && stackFiltering !== 'verbose') {
@@ -42,9 +48,9 @@ export default function tameErrorConstructor(
}
const ErrorPrototype = FERAL_ERROR.prototype;

- const platform =
- typeof FERAL_ERROR.captureStackTrace === 'function' ? 'v8' : 'unknown';
const { captureStackTrace: originalCaptureStackTrace } = FERAL_ERROR;
+ const platform =
+ typeof originalCaptureStackTrace === 'function' ? 'v8' : 'unknown';

const makeErrorConstructor = (_ = {}) => {
// eslint-disable-next-line no-shadow
@@ -122,6 +128,45 @@ export default function tameErrorConstructor(
},
});

+ if (errorTaming === 'unsafe-debug' && platform === 'v8') {
+ // This case is a kludge to work around
+ // https://github.com/endojs/endo/issues/1798
+ // https://github.com/endojs/endo/issues/2348
+ // https://github.com/Agoric/agoric-sdk/issues/8662
+
+ defineProperties(InitialError, {
+ prepareStackTrace: {
+ get() {
+ return FERAL_ERROR.prepareStackTrace;
+ },
+ set(newPrepareStackTrace) {
+ FERAL_ERROR.prepareStackTrace = newPrepareStackTrace;
+ },
+ enumerable: false,
+ configurable: true,
+ },
+ captureStackTrace: {
+ value: FERAL_ERROR.captureStackTrace,
+ writable: true,
+ enumerable: false,
+ configurable: true,
+ },
+ });
+
+ const descs = getOwnPropertyDescriptors(InitialError);
+ defineProperties(SharedError, {
+ stackTraceLimit: descs.stackTraceLimit,
+ prepareStackTrace: descs.prepareStackTrace,
+ captureStackTrace: descs.captureStackTrace,
+ });
+
+ return {
+ '%InitialGetStackString%': initialGetStackString,
+ '%InitialError%': InitialError,
+ '%SharedError%': SharedError,
+ };
+ }
+
// The default SharedError much be completely powerless even on v8,
// so the lenient `stackTraceLimit` accessor does nothing on all
// platforms.
@@ -171,7 +216,6 @@ export default function tameErrorConstructor(
});
}

- let initialGetStackString = tamedMethods.getStackString;
if (platform === 'v8') {
initialGetStackString = tameV8ErrorConstructor(
FERAL_ERROR,
@@ -179,7 +223,7 @@ export default function tameErrorConstructor(
errorTaming,
stackFiltering,
);
- } else if (errorTaming === 'unsafe') {
+ } else if (errorTaming === 'unsafe' || errorTaming === 'unsafe-debug') {
// v8 has too much magic around their 'stack' own property for it to
// coexist cleanly with this accessor. So only install it on non-v8

diff --git a/node_modules/ses/src/lockdown.js b/node_modules/ses/src/lockdown.js
index 107b5d0..dd709e5 100644
--- a/node_modules/ses/src/lockdown.js
+++ b/node_modules/ses/src/lockdown.js
@@ -58,7 +58,7 @@ import { tameFauxDataProperties } from './tame-faux-data-properties.js';

/** @import {LockdownOptions} from '../types.js' */

-const { Fail, details: d, quote: q } = assert;
+const { Fail, details: X, quote: q } = assert;

/** @type {Error=} */
let priorRepairIntrinsics;
@@ -200,7 +200,7 @@ export const repairIntrinsics = (options = {}) => {
priorRepairIntrinsics === undefined ||
// eslint-disable-next-line @endo/no-polymorphic-call
assert.fail(
- d`Already locked down at ${priorRepairIntrinsics} (SES_ALREADY_LOCKED_DOWN)`,
+ X`Already locked down at ${priorRepairIntrinsics} (SES_ALREADY_LOCKED_DOWN)`,
TypeError,
);
// See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_ALREADY_LOCKED_DOWN.md
@@ -298,7 +298,7 @@ export const repairIntrinsics = (options = {}) => {
* @type {((error: any) => string | undefined) | undefined}
*/
let optGetStackString;
- if (errorTaming !== 'unsafe') {
+ if (errorTaming === 'safe') {
optGetStackString = intrinsics['%InitialGetStackString%'];
}
const consoleRecord = tameConsole(
@@ -318,13 +318,17 @@ export const repairIntrinsics = (options = {}) => {
// There doesn't seem to be a cleaner way to reach it.
hostIntrinsics.SafeMap = getPrototypeOf(
// eslint-disable-next-line no-underscore-dangle
- /** @type {any} */ (consoleRecord.console)._times,
+ /** @type {any} */(consoleRecord.console)._times,
);
}

// @ts-ignore assert is absent on globalThis type def.
- if (errorTaming === 'unsafe' && globalThis.assert === assert) {
- // If errorTaming is 'unsafe' we replace the global assert with
+ if (
+ (errorTaming === 'unsafe' || errorTaming === 'unsafe-debug') &&
+ globalThis.assert === assert
+ ) {
+ // If errorTaming is 'unsafe' or 'unsafe-debug' we replace the
+ // global assert with
// one whose `details` template literal tag does not redact
// unmarked substitution values. IOW, it blabs information that
// was supposed to be secret from callers, as an aid to debugging
@@ -391,7 +395,7 @@ export const repairIntrinsics = (options = {}) => {
priorHardenIntrinsics === undefined ||
// eslint-disable-next-line @endo/no-polymorphic-call
assert.fail(
- d`Already locked down at ${priorHardenIntrinsics} (SES_ALREADY_LOCKED_DOWN)`,
+ X`Already locked down at ${priorHardenIntrinsics} (SES_ALREADY_LOCKED_DOWN)`,
TypeError,
);
// See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_ALREADY_LOCKED_DOWN.md
Loading