Skip to content

Commit

Permalink
esm: handle more error types thrown from the loader thread
Browse files Browse the repository at this point in the history
PR-URL: nodejs#48247
Refs: nodejs#48240
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jacob Smith <jacob@frende.me>
  • Loading branch information
aduh95 authored and targos committed Nov 10, 2023
1 parent a31ca4e commit 2bb6dfb
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 6 deletions.
11 changes: 6 additions & 5 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -589,16 +589,17 @@ class HooksProxy {
if (response.message.status === 'never-settle') {
return new Promise(() => {});
}
if (response.message.status === 'error') {
if (response.message.body == null) throw response.message.body;
if (response.message.body.serializationFailed || response.message.body.serialized == null) {
const { status, body } = response.message;
if (status === 'error') {
if (body == null || typeof body !== 'object') throw body;
if (body.serializationFailed || body.serialized == null) {
throw ERR_WORKER_UNSERIALIZABLE_ERROR();
}

// eslint-disable-next-line no-restricted-syntax
throw deserializeError(response.message.body.serialized);
throw deserializeError(body.serialized);
} else {
return response.message.body;
return body;
}
}

Expand Down
5 changes: 4 additions & 1 deletion lib/internal/modules/esm/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ function transferArrayBuffer(hasError, source) {
}

function wrapMessage(status, body) {
if (status === 'success' || body === null || typeof body !== 'object') {
if (status === 'success' || body === null ||
(typeof body !== 'object' &&
typeof body !== 'function' &&
typeof body !== 'symbol')) {
return { status, body };
}

Expand Down
156 changes: 156 additions & 0 deletions test/es-module/test-esm-loader-hooks.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,162 @@ describe('Loader hooks', { concurrency: true }, () => {
assert.strictEqual(signal, null);
});

describe('should handle a throwing top-level body', () => {
it('should handle regular Error object', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw new Error("error message")',
fixtures.path('empty.js'),
]);

assert.match(stderr, /Error: error message\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle null', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw null',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\nnull\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle undefined', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw undefined',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\nundefined\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle boolean', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw true',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\ntrue\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle empty plain object', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw {}',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\n\{\}\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle plain object', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw {fn(){},symbol:Symbol("symbol"),u:undefined}',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\n\{ fn: \[Function: fn\], symbol: Symbol\(symbol\), u: undefined \}\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle number', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw 1',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\n1\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle bigint', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw 1n',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\n1\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle string', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw "literal string"',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\nliteral string\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle symbol', async () => {
const { code, signal, stdout } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw Symbol("symbol descriptor")',
fixtures.path('empty.js'),
]);

// Throwing a symbol doesn't produce any output
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

it('should handle function', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
'data:text/javascript,throw function fnName(){}',
fixtures.path('empty.js'),
]);

assert.match(stderr, /\n\[Function: fnName\]\r?\n/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});
});

it('should be fine to call `process.removeAllListeners("beforeExit")` from the main thread', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
Expand Down

0 comments on commit 2bb6dfb

Please sign in to comment.