Skip to content

Commit

Permalink
feat(ses): errorTrapping reports prepend "SES_UNCAUGHT_EXCEPTION:" (
Browse files Browse the repository at this point in the history
#2402)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

Use `"SES_UNCAUGHT_EXCEPTION: ...` report output for parity with
`SES_UNHANDLED_REJECTION: ...` report output. Without this feature,
`ses` would write the naked exception to stderr, which could lead to
confusion when trying to understand _why_ the output was written. This
was doubly confusing if the exception was not an `Error`, rather just
some other thrown value.

### Security Considerations

Improves security by giving more context for error output.

### Scaling Considerations

n/a

### Documentation Considerations

n/a

### Testing Considerations

Just unit tests.

### Compatibility Considerations

Since stderr is not generally parsed by programs, I am not concerned
with breaking compatibility of the rendering of the uncaught exceptions.
This change should improve the user experience.

### Upgrade Considerations

If the stderr of your SES program is processed by another program, this
change will be visible. You may need to change your error processor to
be aware of `SES_UNCAUGHT_EXCEPTION: ...` output on stderr.
  • Loading branch information
michaelfig authored Aug 16, 2024
2 parents bec206b + 6500799 commit aaec188
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 13 deletions.
15 changes: 15 additions & 0 deletions packages/ses/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
User-visible changes in `ses`:

# Unreleased

- If lockdown's `errorTrapping: 'report'` mode is selected (possibly via the
`'platform'`, or `'exit'` or `'abort'` modes), uncaught exceptions will be
written to standard error with the new `SES_UNCAUGHT_EXCEPTION: ` prefix.
This is intended to give valuable context to users of the system, especially
when an uncaught exception is not an `Error` object, and therefore its origin
may be hard to find in source code.

This is not likely to affect most systems built with SES, as stderr is
generally reserved for user-only messages. If your SES system sends its
stderr to a program which parses it, you may need to adapt that program to be
tolerant of the `SES_UNCAUGHT_EXCEPTION: ` prefix. Even for such programs, it
is unlikely they are that sensitive to stderr formatting.

# v1.6.0 (2024-07-30)

- *NOTICE*: This version introduces multiple features to converge upon a
Expand Down
2 changes: 1 addition & 1 deletion packages/ses/error-codes/SES_ALREADY_LOCKED_DOWN.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ was called.

```
(TypeError#1)
TypeError#1: Already lockded down (SES_ALREADY_LOCKED_DOWN)
TypeError#1: Already locked down (SES_ALREADY_LOCKED_DOWN)
at repairIntrinsics
at lockdown
Expand Down
11 changes: 11 additions & 0 deletions packages/ses/error-codes/SES_UNCAUGHT_EXCEPTION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Error output beginning with `SES_UNCAUGHT_EXCEPTION:`

The SES shim provides options for management of exceptions that are not caught by any user code.

This behavior is configured by calling `lockdown` with the [errorTrapping][] option. If that option is explicitly set to `'none'`, the platform's default error trap remains in effect.

Otherwise, when thrown exceptions have no `catch` handler, the SES shim will print `SES_UNCAUGHT_EXCEPTION: exception...` to the error console. In that case, `exception...` is the thrown exception, augmented with causal error information when possible.

For most programs, the error console is intended for human consumption, and so the `SES_UNCAUGHT_EXCEPTION:` output can be useful in giving a clue as to the source of a failure.

[errorTrapping]: ../docs/lockdown.md#errortrapping-options
11 changes: 11 additions & 0 deletions packages/ses/error-codes/SES_UNHANDLED_REJECTION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Error output beginning with `SES_UNHANDLED_REJECTION:`

The SES shim provides options for management of rejected promises that have not had a handler attached before the promise was garbage-collected.

This behavior is configured by calling `lockdown` with the [unhandledRejectionTrapping][] option. If that option is explicitly set to `'none'`, the platform's default unhandled rejection trap remains in effect.

Otherwise, when unhandled rejections are detected, the SES shim will print `SES_UNHANDLED_REJECTION: reason...` to the error console. In that case, `reason...` is the rejection reason, augmented with causal error information when possible.

For most programs, the error console is intended for human consumption, and so the `SES_UNHANDLED_REJECTION:` output can be useful in giving a clue as to the source of a failure.

[unhandledRejectionTrapping]: ../docs/lockdown.md#unhandledrejectiontrapping-options
11 changes: 7 additions & 4 deletions packages/ses/src/error/tame-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ export const tameConsole = (
}

globalProcess.on('uncaughtException', error => {
// causalConsole is born frozen so not vulnerable to method tampering.
ourConsole.error(error);
// See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_UNCAUGHT_EXCEPTION.md
ourConsole.error('SES_UNCAUGHT_EXCEPTION:', error);
if (terminate) {
terminate();
}
Expand All @@ -133,8 +133,9 @@ export const tameConsole = (
typeof globalProcess.on === 'function'
) {
const handleRejection = reason => {
// 'platform' and 'report' just log the reason.
// See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_UNHANDLED_REJECTION.md
ourConsole.error('SES_UNHANDLED_REJECTION:', reason);
// 'platform' and 'report' just log the reason.
};
// Maybe track unhandled promise rejections.
const h = makeRejectionHandlers(handleRejection);
Expand All @@ -155,8 +156,9 @@ export const tameConsole = (
) {
globalWindow.addEventListener('error', event => {
event.preventDefault();
// See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_UNCAUGHT_EXCEPTION.md
ourConsole.error('SES_UNCAUGHT_EXCEPTION:', event.error);
// 'platform' and 'report' just log the reason.
ourConsole.error(event.error);
if (errorTrapping === 'exit' || errorTrapping === 'abort') {
globalWindow.location.href = `about:blank`;
}
Expand All @@ -168,6 +170,7 @@ export const tameConsole = (
typeof globalWindow.addEventListener === 'function'
) {
const handleRejection = reason => {
// See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_UNHANDLED_REJECTION.md
ourConsole.error('SES_UNHANDLED_REJECTION:', reason);
};

Expand Down
20 changes: 14 additions & 6 deletions packages/ses/test/console-error-trap.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ const exitAssertions = (
code === expectedCode || code === altExpectedCode,
'exit error code',
);
t.assert(
stderr.includes('SES_UNCAUGHT_EXCEPTION:'),
'stderr should have SES_UNCAUGHT_EXCEPTION:',
);
t.assert(
stderr.includes('(Error#1)'),
'stderr should have an error marker',
Expand All @@ -44,47 +48,51 @@ const exitAssertions = (
};

test('errors reveal their stacks', async t => {
t.plan(5);
t.plan(6);
await new Promise(resolve => {
exec('node default.js', { cwd }, exitAssertions(t, resolve, 255));
});
});

test('errors reveal their stacks with errorTrapping: platform', async t => {
t.plan(5);
t.plan(6);
await new Promise(resolve => {
exec('node platform.js', { cwd }, exitAssertions(t, resolve, 255));
});
});

test('errors reveal their stacks with errorTrapping: exit', async t => {
t.plan(5);
t.plan(6);
await new Promise(resolve => {
exec('node exit.js', { cwd }, exitAssertions(t, resolve, 255));
});
});

test('errors reveal their stacks with errorTrapping: exit with code', async t => {
t.plan(5);
t.plan(6);
await new Promise(resolve => {
exec('node exit-code.js', { cwd }, exitAssertions(t, resolve, 127));
});
});

test('errors reveal their stacks with errorTrapping: abort', async t => {
t.plan(5);
t.plan(6);
// Mac exits with null, Linux exits with code 134
await new Promise(resolve => {
exec('node abort.js', { cwd }, exitAssertions(t, resolve, null, 134));
});
});

test('errors reveal their stacks with errorTrapping: report', async t => {
t.plan(5);
t.plan(6);
await new Promise(resolve => {
exec('node report.js', { cwd }, (err, stdout, stderr) => {
t.log({ stdout, stderr });
t.is(err, null);
t.assert(
stderr.includes('SES_UNCAUGHT_EXCEPTION:'),
'stderr should have SES_UNCAUGHT_EXCEPTION:',
);
t.assert(
stderr.includes('(Error#1)'),
'stderr should have an error marker',
Expand Down
4 changes: 2 additions & 2 deletions packages/ses/test/console-rejection-trap.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const exitAssertions = (
'stderr should have an error marker',
);
t.assert(
code !== 0 || stderr.includes('SES_UNHANDLED_REJECTION'),
'stderr should have SES_UNHANDLED_REJECTION',
code !== 0 || stderr.includes('SES_UNHANDLED_REJECTION:'),
'stderr should have SES_UNHANDLED_REJECTION:',
);
t.assert(
code === 0 || !stderr.includes('(Error#2)'),
Expand Down

0 comments on commit aaec188

Please sign in to comment.