Skip to content

Commit

Permalink
worker: fix nested uncaught exception handling
Browse files Browse the repository at this point in the history
We are using `ObjectPrototypeToString()` as a cross-context brand check
for built-in errors, but weren’t making sure to set that when
deserializing errors back into JS objects.

Fix that by setting `[Symbol.toStringTag]` manually, to make sure that
multiple serialize-and-deserialize cycles keep giving the same result.

Fixes: #34309

PR-URL: #34310
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax committed Sep 22, 2020
1 parent ed65137 commit c31b6bf
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
5 changes: 5 additions & 0 deletions lib/internal/error-serdes.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const {
ObjectKeys,
ObjectPrototypeToString,
SafeSet,
SymbolToStringTag,
} = primordials;

const kSerializedError = 0;
Expand Down Expand Up @@ -116,6 +117,10 @@ function deserializeError(error) {
case kSerializedError:
const { constructor, properties } = deserialize(error.subarray(1));
const ctor = errors[constructor];
ObjectDefineProperty(properties, SymbolToStringTag, {
value: { value: 'Error', configurable: true },
enumerable: true
});
return ObjectCreate(ctor.prototype, properties);
case kSerializedObject:
return deserialize(error.subarray(1));
Expand Down
9 changes: 7 additions & 2 deletions test/parallel/test-error-serdes.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@ assert.strictEqual(cycle(null), null);
assert.strictEqual(cycle(undefined), undefined);
assert.strictEqual(cycle('foo'), 'foo');

{
const err = cycle(new Error('foo'));
let err = new Error('foo');
for (let i = 0; i < 10; i++) {
assert(err instanceof Error);
assert(Object.prototype.toString.call(err), '[object Error]');
assert.strictEqual(err.name, 'Error');
assert.strictEqual(err.message, 'foo');
assert(/^Error: foo\n/.test(err.stack));

const prev = err;
err = cycle(err);
assert.deepStrictEqual(err, prev);
}

assert.strictEqual(cycle(new RangeError('foo')).name, 'RangeError');
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-worker-nested-uncaught.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';
const common = require('../common');
const { Worker } = require('worker_threads');

// Regression test for https://github.com/nodejs/node/issues/34309

const w = new Worker(
`const { Worker } = require('worker_threads');
new Worker("throw new Error('uncaught')", { eval:true })`,
{ eval: true });
w.on('error', common.expectsError({
name: 'Error',
message: 'uncaught'
}));

0 comments on commit c31b6bf

Please sign in to comment.