Skip to content

Commit 02041f8

Browse files
serchersercher
committed
esm: handle more error types thrown from the loader thread
PR-URL: nodejs/node#48247 Backport-PR-URL: nodejs/node#50669 Refs: nodejs/node#48240 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
1 parent 721b5d9 commit 02041f8

File tree

3 files changed

+166
-6
lines changed

3 files changed

+166
-6
lines changed

‎graal-nodejs/lib/internal/modules/esm/hooks.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -589,16 +589,17 @@ class HooksProxy {
589589
if (response.message.status === 'never-settle') {
590590
return new Promise(() => {});
591591
}
592-
if (response.message.status === 'error') {
593-
if (response.message.body == null) throw response.message.body;
594-
if (response.message.body.serializationFailed || response.message.body.serialized == null) {
592+
const { status, body } = response.message;
593+
if (status === 'error') {
594+
if (body == null || typeof body !== 'object') throw body;
595+
if (body.serializationFailed || body.serialized == null) {
595596
throw ERR_WORKER_UNSERIALIZABLE_ERROR();
596597
}
597598

598599
// eslint-disable-next-line no-restricted-syntax
599-
throw deserializeError(response.message.body.serialized);
600+
throw deserializeError(body.serialized);
600601
} else {
601-
return response.message.body;
602+
return body;
602603
}
603604
}
604605

‎graal-nodejs/lib/internal/modules/esm/worker.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ function transferArrayBuffer(hasError, source) {
4040
}
4141

4242
function wrapMessage(status, body) {
43-
if (status === 'success' || body === null || typeof body !== 'object') {
43+
if (status === 'success' || body === null ||
44+
(typeof body !== 'object' &&
45+
typeof body !== 'function' &&
46+
typeof body !== 'symbol')) {
4447
return { status, body };
4548
}
4649

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

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,162 @@ describe('Loader hooks', { concurrency: true }, () => {
192192
assert.strictEqual(signal, null);
193193
});
194194

195+
describe('should handle a throwing top-level body', () => {
196+
it('should handle regular Error object', async () => {
197+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
198+
'--no-warnings',
199+
'--experimental-loader',
200+
'data:text/javascript,throw new Error("error message")',
201+
fixtures.path('empty.js'),
202+
]);
203+
204+
assert.match(stderr, /Error: error message\r?\n/);
205+
assert.strictEqual(stdout, '');
206+
assert.strictEqual(code, 1);
207+
assert.strictEqual(signal, null);
208+
});
209+
210+
it('should handle null', async () => {
211+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
212+
'--no-warnings',
213+
'--experimental-loader',
214+
'data:text/javascript,throw null',
215+
fixtures.path('empty.js'),
216+
]);
217+
218+
assert.match(stderr, /\nnull\r?\n/);
219+
assert.strictEqual(stdout, '');
220+
assert.strictEqual(code, 1);
221+
assert.strictEqual(signal, null);
222+
});
223+
224+
it('should handle undefined', async () => {
225+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
226+
'--no-warnings',
227+
'--experimental-loader',
228+
'data:text/javascript,throw undefined',
229+
fixtures.path('empty.js'),
230+
]);
231+
232+
assert.match(stderr, /\nundefined\r?\n/);
233+
assert.strictEqual(stdout, '');
234+
assert.strictEqual(code, 1);
235+
assert.strictEqual(signal, null);
236+
});
237+
238+
it('should handle boolean', async () => {
239+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
240+
'--no-warnings',
241+
'--experimental-loader',
242+
'data:text/javascript,throw true',
243+
fixtures.path('empty.js'),
244+
]);
245+
246+
assert.match(stderr, /\ntrue\r?\n/);
247+
assert.strictEqual(stdout, '');
248+
assert.strictEqual(code, 1);
249+
assert.strictEqual(signal, null);
250+
});
251+
252+
it('should handle empty plain object', async () => {
253+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
254+
'--no-warnings',
255+
'--experimental-loader',
256+
'data:text/javascript,throw {}',
257+
fixtures.path('empty.js'),
258+
]);
259+
260+
assert.match(stderr, /\n\{\}\r?\n/);
261+
assert.strictEqual(stdout, '');
262+
assert.strictEqual(code, 1);
263+
assert.strictEqual(signal, null);
264+
});
265+
266+
it('should handle plain object', async () => {
267+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
268+
'--no-warnings',
269+
'--experimental-loader',
270+
'data:text/javascript,throw {fn(){},symbol:Symbol("symbol"),u:undefined}',
271+
fixtures.path('empty.js'),
272+
]);
273+
274+
assert.match(stderr, /\n\{ fn: \[Function: fn\], symbol: Symbol\(symbol\), u: undefined \}\r?\n/);
275+
assert.strictEqual(stdout, '');
276+
assert.strictEqual(code, 1);
277+
assert.strictEqual(signal, null);
278+
});
279+
280+
it('should handle number', async () => {
281+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
282+
'--no-warnings',
283+
'--experimental-loader',
284+
'data:text/javascript,throw 1',
285+
fixtures.path('empty.js'),
286+
]);
287+
288+
assert.match(stderr, /\n1\r?\n/);
289+
assert.strictEqual(stdout, '');
290+
assert.strictEqual(code, 1);
291+
assert.strictEqual(signal, null);
292+
});
293+
294+
it('should handle bigint', async () => {
295+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
296+
'--no-warnings',
297+
'--experimental-loader',
298+
'data:text/javascript,throw 1n',
299+
fixtures.path('empty.js'),
300+
]);
301+
302+
assert.match(stderr, /\n1\r?\n/);
303+
assert.strictEqual(stdout, '');
304+
assert.strictEqual(code, 1);
305+
assert.strictEqual(signal, null);
306+
});
307+
308+
it('should handle string', async () => {
309+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
310+
'--no-warnings',
311+
'--experimental-loader',
312+
'data:text/javascript,throw "literal string"',
313+
fixtures.path('empty.js'),
314+
]);
315+
316+
assert.match(stderr, /\nliteral string\r?\n/);
317+
assert.strictEqual(stdout, '');
318+
assert.strictEqual(code, 1);
319+
assert.strictEqual(signal, null);
320+
});
321+
322+
it('should handle symbol', async () => {
323+
const { code, signal, stdout } = await spawnPromisified(execPath, [
324+
'--no-warnings',
325+
'--experimental-loader',
326+
'data:text/javascript,throw Symbol("symbol descriptor")',
327+
fixtures.path('empty.js'),
328+
]);
329+
330+
// Throwing a symbol doesn't produce any output
331+
assert.strictEqual(stdout, '');
332+
assert.strictEqual(code, 1);
333+
assert.strictEqual(signal, null);
334+
});
335+
336+
it('should handle function', async () => {
337+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
338+
'--no-warnings',
339+
'--experimental-loader',
340+
'data:text/javascript,throw function fnName(){}',
341+
fixtures.path('empty.js'),
342+
]);
343+
344+
assert.match(stderr, /\n\[Function: fnName\]\r?\n/);
345+
assert.strictEqual(stdout, '');
346+
assert.strictEqual(code, 1);
347+
assert.strictEqual(signal, null);
348+
});
349+
});
350+
195351
it('should be fine to call `process.removeAllListeners("beforeExit")` from the main thread', async () => {
196352
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
197353
'--no-warnings',

0 commit comments

Comments
 (0)