Skip to content

Commit 215acb4

Browse files
aduh95Ceres6
aduh95
authored and
Ceres6
committed
esm: handle more error types thrown from the loader thread
PR-URL: nodejs#48247 Refs: nodejs#48240 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
1 parent 4b44321 commit 215acb4

File tree

3 files changed

+166
-6
lines changed

3 files changed

+166
-6
lines changed

‎lib/internal/modules/esm/hooks.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -590,16 +590,17 @@ class HooksProxy {
590590
if (response.message.status === 'never-settle') {
591591
return new Promise(() => {});
592592
}
593-
if (response.message.status === 'error') {
594-
if (response.message.body == null) throw response.message.body;
595-
if (response.message.body.serializationFailed || response.message.body.serialized == null) {
593+
const { status, body } = response.message;
594+
if (status === 'error') {
595+
if (body == null || typeof body !== 'object') throw body;
596+
if (body.serializationFailed || body.serialized == null) {
596597
throw ERR_WORKER_UNSERIALIZABLE_ERROR();
597598
}
598599

599600
// eslint-disable-next-line no-restricted-syntax
600-
throw deserializeError(response.message.body.serialized);
601+
throw deserializeError(body.serialized);
601602
} else {
602-
return response.message.body;
603+
return body;
603604
}
604605
}
605606

‎lib/internal/modules/esm/worker.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ function transferArrayBuffer(hasError, source) {
4848
}
4949

5050
function wrapMessage(status, body) {
51-
if (status === 'success' || body === null || typeof body !== 'object') {
51+
if (status === 'success' || body === null ||
52+
(typeof body !== 'object' &&
53+
typeof body !== 'function' &&
54+
typeof body !== 'symbol')) {
5255
return { status, body };
5356
}
5457

‎test/es-module/test-esm-loader-hooks.mjs

+156
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,162 @@ describe('Loader hooks', { concurrency: true }, () => {
244244
assert.strictEqual(signal, null);
245245
});
246246

247+
describe('should handle a throwing top-level body', () => {
248+
it('should handle regular Error object', async () => {
249+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
250+
'--no-warnings',
251+
'--experimental-loader',
252+
'data:text/javascript,throw new Error("error message")',
253+
fixtures.path('empty.js'),
254+
]);
255+
256+
assert.match(stderr, /Error: error message\r?\n/);
257+
assert.strictEqual(stdout, '');
258+
assert.strictEqual(code, 1);
259+
assert.strictEqual(signal, null);
260+
});
261+
262+
it('should handle null', async () => {
263+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
264+
'--no-warnings',
265+
'--experimental-loader',
266+
'data:text/javascript,throw null',
267+
fixtures.path('empty.js'),
268+
]);
269+
270+
assert.match(stderr, /\nnull\r?\n/);
271+
assert.strictEqual(stdout, '');
272+
assert.strictEqual(code, 1);
273+
assert.strictEqual(signal, null);
274+
});
275+
276+
it('should handle undefined', async () => {
277+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
278+
'--no-warnings',
279+
'--experimental-loader',
280+
'data:text/javascript,throw undefined',
281+
fixtures.path('empty.js'),
282+
]);
283+
284+
assert.match(stderr, /\nundefined\r?\n/);
285+
assert.strictEqual(stdout, '');
286+
assert.strictEqual(code, 1);
287+
assert.strictEqual(signal, null);
288+
});
289+
290+
it('should handle boolean', async () => {
291+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
292+
'--no-warnings',
293+
'--experimental-loader',
294+
'data:text/javascript,throw true',
295+
fixtures.path('empty.js'),
296+
]);
297+
298+
assert.match(stderr, /\ntrue\r?\n/);
299+
assert.strictEqual(stdout, '');
300+
assert.strictEqual(code, 1);
301+
assert.strictEqual(signal, null);
302+
});
303+
304+
it('should handle empty plain object', async () => {
305+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
306+
'--no-warnings',
307+
'--experimental-loader',
308+
'data:text/javascript,throw {}',
309+
fixtures.path('empty.js'),
310+
]);
311+
312+
assert.match(stderr, /\n\{\}\r?\n/);
313+
assert.strictEqual(stdout, '');
314+
assert.strictEqual(code, 1);
315+
assert.strictEqual(signal, null);
316+
});
317+
318+
it('should handle plain object', async () => {
319+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
320+
'--no-warnings',
321+
'--experimental-loader',
322+
'data:text/javascript,throw {fn(){},symbol:Symbol("symbol"),u:undefined}',
323+
fixtures.path('empty.js'),
324+
]);
325+
326+
assert.match(stderr, /\n\{ fn: \[Function: fn\], symbol: Symbol\(symbol\), u: undefined \}\r?\n/);
327+
assert.strictEqual(stdout, '');
328+
assert.strictEqual(code, 1);
329+
assert.strictEqual(signal, null);
330+
});
331+
332+
it('should handle number', async () => {
333+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
334+
'--no-warnings',
335+
'--experimental-loader',
336+
'data:text/javascript,throw 1',
337+
fixtures.path('empty.js'),
338+
]);
339+
340+
assert.match(stderr, /\n1\r?\n/);
341+
assert.strictEqual(stdout, '');
342+
assert.strictEqual(code, 1);
343+
assert.strictEqual(signal, null);
344+
});
345+
346+
it('should handle bigint', async () => {
347+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
348+
'--no-warnings',
349+
'--experimental-loader',
350+
'data:text/javascript,throw 1n',
351+
fixtures.path('empty.js'),
352+
]);
353+
354+
assert.match(stderr, /\n1\r?\n/);
355+
assert.strictEqual(stdout, '');
356+
assert.strictEqual(code, 1);
357+
assert.strictEqual(signal, null);
358+
});
359+
360+
it('should handle string', async () => {
361+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
362+
'--no-warnings',
363+
'--experimental-loader',
364+
'data:text/javascript,throw "literal string"',
365+
fixtures.path('empty.js'),
366+
]);
367+
368+
assert.match(stderr, /\nliteral string\r?\n/);
369+
assert.strictEqual(stdout, '');
370+
assert.strictEqual(code, 1);
371+
assert.strictEqual(signal, null);
372+
});
373+
374+
it('should handle symbol', async () => {
375+
const { code, signal, stdout } = await spawnPromisified(execPath, [
376+
'--no-warnings',
377+
'--experimental-loader',
378+
'data:text/javascript,throw Symbol("symbol descriptor")',
379+
fixtures.path('empty.js'),
380+
]);
381+
382+
// Throwing a symbol doesn't produce any output
383+
assert.strictEqual(stdout, '');
384+
assert.strictEqual(code, 1);
385+
assert.strictEqual(signal, null);
386+
});
387+
388+
it('should handle function', async () => {
389+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
390+
'--no-warnings',
391+
'--experimental-loader',
392+
'data:text/javascript,throw function fnName(){}',
393+
fixtures.path('empty.js'),
394+
]);
395+
396+
assert.match(stderr, /\n\[Function: fnName\]\r?\n/);
397+
assert.strictEqual(stdout, '');
398+
assert.strictEqual(code, 1);
399+
assert.strictEqual(signal, null);
400+
});
401+
});
402+
247403
it('should be fine to call `process.removeAllListeners("beforeExit")` from the main thread', async () => {
248404
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
249405
'--no-warnings',

0 commit comments

Comments
 (0)