From d51085260652096f4259612de5aba517c6aae954 Mon Sep 17 00:00:00 2001
From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Date: Fri, 24 Jun 2022 11:38:54 +0200
Subject: [PATCH] fixup: add test case for `null` hook returns

---
 lib/internal/modules/esm/loader.js            | 14 +++---
 test/es-module/test-esm-loader-chaining.mjs   | 46 ++++++++++++++++++-
 .../loader-load-null-return.mjs               |  3 ++
 .../loader-resolve-null-return.mjs            |  3 ++
 4 files changed, 56 insertions(+), 10 deletions(-)
 create mode 100644 test/fixtures/es-module-loaders/loader-load-null-return.mjs
 create mode 100644 test/fixtures/es-module-loaders/loader-resolve-null-return.mjs

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index 10f53ba6851f5d..d4f2bc7f5b5b01 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -94,6 +94,8 @@ const { getOptionValue } = require('internal/options');
 
 let emittedSpecifierResolutionWarning = false;
 
+const nullTypeForErr = { constructor: { name: 'null' } };
+
 /**
  * A utility function to iterate through a hook chain, track advancement in the
  * chain, and generate and supply the `next<HookName>` argument to the custom
@@ -602,11 +604,11 @@ class ESMLoader {
       if (ctx) validateObject(ctx, `${hookErrIdentifier} context`);
     };
     const validateOutput = (hookErrIdentifier, output) => {
-      if (typeof output !== 'object') { // [2]
+      if (typeof output !== 'object' || output === null) { // [2]
         throw new ERR_INVALID_RETURN_VALUE(
           'an object',
           hookErrIdentifier,
-          output,
+          output === null ? nullTypeForErr : output,
         );
       }
     };
@@ -838,15 +840,11 @@ class ESMLoader {
       if (ctx) validateObject(ctx, `${hookErrIdentifier} context`);
     };
     const validateOutput = (hookErrIdentifier, output) => {
-      if (
-        typeof output !== 'object' || // [2]
-        output === null ||
-        (output.url == null && typeof output.then === 'function')
-      ) {
+      if (typeof output !== 'object' || output === null) { // [2]
         throw new ERR_INVALID_RETURN_VALUE(
           'an object',
           hookErrIdentifier,
-          output,
+          output === null ? nullTypeForErr : output,
         );
       }
     };
diff --git a/test/es-module/test-esm-loader-chaining.mjs b/test/es-module/test-esm-loader-chaining.mjs
index 977c20dbbbab37..5b8bc65eeeec33 100644
--- a/test/es-module/test-esm-loader-chaining.mjs
+++ b/test/es-module/test-esm-loader-chaining.mjs
@@ -229,7 +229,7 @@ const commonArgs = [
   assert.strictEqual(status, 0);
 }
 
-{ // Verify chain does break and throws appropriately
+{ // Verify resolve chain does break and throws appropriately
   const { status, stderr, stdout } = spawnSync(
     process.execPath,
     [
@@ -273,7 +273,7 @@ const commonArgs = [
   assert.strictEqual(status, 1);
 }
 
-{ // Verify chain does break and throws appropriately
+{ // Verify load chain does break and throws appropriately
   const { status, stderr, stdout } = spawnSync(
     process.execPath,
     [
@@ -314,6 +314,27 @@ const commonArgs = [
   assert.match(stderr, /'resolve' hook's nextResolve\(\) specifier/);
 }
 
+{ // Verify error thrown when resolve hook is invalid
+  const { status, stderr } = spawnSync(
+    process.execPath,
+    [
+      '--loader',
+      fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'),
+      '--loader',
+      fixtures.fileURL('es-module-loaders', 'loader-resolve-null-return.mjs'),
+      ...commonArgs,
+    ],
+    { encoding: 'utf8' },
+  );
+
+  assert.strictEqual(status, 1);
+  assert.match(stderr, /ERR_INVALID_RETURN_VALUE/);
+  assert.match(stderr, /loader-resolve-null-return\.mjs/);
+  assert.match(stderr, /'resolve' hook's nextResolve\(\)/);
+  assert.match(stderr, /an object/);
+  assert.match(stderr, /instance of null/);
+}
+
 { // Verify error thrown when invalid `context` argument passed to `nextResolve`
   const { status, stderr } = spawnSync(
     process.execPath,
@@ -333,6 +354,27 @@ const commonArgs = [
   assert.strictEqual(status, 1);
 }
 
+{ // Verify error thrown when load hook is invalid
+  const { status, stderr } = spawnSync(
+    process.execPath,
+    [
+      '--loader',
+      fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'),
+      '--loader',
+      fixtures.fileURL('es-module-loaders', 'loader-load-null-return.mjs'),
+      ...commonArgs,
+    ],
+    { encoding: 'utf8' },
+  );
+
+  assert.strictEqual(status, 1);
+  assert.match(stderr, /ERR_INVALID_RETURN_VALUE/);
+  assert.match(stderr, /loader-load-null-return\.mjs/);
+  assert.match(stderr, /'load' hook's nextLoad\(\)/);
+  assert.match(stderr, /an object/);
+  assert.match(stderr, /instance of null/);
+}
+
 { // Verify error thrown when invalid `url` argument passed to `nextLoad`
   const { status, stderr } = spawnSync(
     process.execPath,
diff --git a/test/fixtures/es-module-loaders/loader-load-null-return.mjs b/test/fixtures/es-module-loaders/loader-load-null-return.mjs
new file mode 100644
index 00000000000000..252eec4ebc28a9
--- /dev/null
+++ b/test/fixtures/es-module-loaders/loader-load-null-return.mjs
@@ -0,0 +1,3 @@
+export async function load(specifier, context, next) {
+  return null;
+}
diff --git a/test/fixtures/es-module-loaders/loader-resolve-null-return.mjs b/test/fixtures/es-module-loaders/loader-resolve-null-return.mjs
new file mode 100644
index 00000000000000..16c9826a8ba51e
--- /dev/null
+++ b/test/fixtures/es-module-loaders/loader-resolve-null-return.mjs
@@ -0,0 +1,3 @@
+export async function resolve(specifier, context, next) {
+  return null;
+}