From 9938a8bf13012395dfc830a4142e9d083b174ef7 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 1 Jun 2023 11:21:01 +0200 Subject: [PATCH] esm: handle more error types thrown from the loader thread PR-URL: https://github.com/nodejs/node/pull/48247 Backport-PR-URL: https://github.com/nodejs/node/pull/50669 Refs: https://github.com/nodejs/node/issues/48240 Reviewed-By: Geoffrey Booth Reviewed-By: Moshe Atlow Reviewed-By: Jacob Smith --- lib/internal/modules/esm/hooks.js | 11 +- lib/internal/modules/esm/worker.js | 5 +- test/es-module/test-esm-loader-hooks.mjs | 156 +++++++++++++++++++++++ 3 files changed, 166 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index c6461c32f06465..d7f8528d5f3b9a 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -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; } } diff --git a/lib/internal/modules/esm/worker.js b/lib/internal/modules/esm/worker.js index 616076c98fb627..380906accf6fe9 100644 --- a/lib/internal/modules/esm/worker.js +++ b/lib/internal/modules/esm/worker.js @@ -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 }; } diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 8756859406aa62..e36a6f04258d96 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -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',