esm: handle more error types thrown from the loader thread

PR-URL: #48247 Refs: #48240 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
nodejs · Jun 4, 2023 · 3c7846d · 3c7846d
1 parent 9a03474
commit 3c7846d
Show file tree

Hide file tree

Showing 3 changed files with 166 additions and 6 deletions.
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -590,16 +590,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
@@ -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
@@ -244,6 +244,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',