module: main resolution fix and not found updates

This simplifies the top-level load when ES modules are enabled
as we can entirely delegate the module resolver, which will hand
over to CommonJS where appropriate.

All not found errors are made consistent to throw during resolve
and have the MODULE_NOT_FOUND code.

Includes the test case from #15736

Author: guybedford

lib/internal/loader/Loader.js

@@ -47,6 +47,7 @@ class Loader {
       throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
                                  'parentURL', 'string');
     }
+
     const { url, format } = await this.resolver(specifier, parentURL,
                                                 ModuleRequest.resolve);
 

lib/internal/loader/ModuleRequest.js

@@ -88,7 +88,14 @@ exports.resolve = (specifier, parentURL) => {
     };
   }
 
-  let url = search(specifier, parentURL);
+  let url;
+  try {
+    url = search(specifier, parentURL);
+  } catch (e) {
+    if (e.message && e.message.startsWith('Cannot find module'))
+      e.code = 'MODULE_NOT_FOUND';
+    throw e;
+  }
 
   if (url.protocol !== 'file:') {
     throw new errors.Error('ERR_INVALID_PROTOCOL',

lib/module.js

@@ -424,39 +424,28 @@ Module._load = function(request, parent, isMain) {
     debug('Module._load REQUEST %s parent: %s', request, parent.id);
   }
 
-  var filename = null;
-
-  if (isMain) {
-    try {
-      filename = Module._resolveFilename(request, parent, isMain);
-    } catch (e) {
-      // try to keep stack
-      e.stack;
-      throw e;
-    }
-    if (experimentalModules) {
-      (async () => {
-        // loader setup
-        if (!ESMLoader) {
-          ESMLoader = new Loader();
-          const userLoader = process.binding('config').userLoader;
-          if (userLoader) {
-            const hooks = await new Loader().import(userLoader);
-            ESMLoader.hook(hooks);
-          }
+  if (isMain && experimentalModules) {
+    (async () => {
+      // loader setup
+      if (!ESMLoader) {
+        ESMLoader = new Loader();
+        const userLoader = process.binding('config').userLoader;
+        if (userLoader) {
+          const hooks = await new Loader().import(userLoader);
+          ESMLoader.hook(hooks);
         }
-        await ESMLoader.import(getURLFromFilePath(filename).href);
-      })()
-      .catch((e) => {
-        console.error(e);
-        process.exit(1);
-      });
-      return;
-    }
-  } else {
-    filename = Module._resolveFilename(request, parent, isMain);
+      }
+      await ESMLoader.import(getURLFromFilePath(request).href);
+    })()
+    .catch((e) => {
+      console.error(e);
+      process.exit(1);
+    });
+    return;
   }
 
+  var filename = Module._resolveFilename(request, parent, isMain);
+
   var cachedModule = Module._cache[filename];
   if (cachedModule) {
     updateChildren(parent, cachedModule, true);

src/module_wrap.cc

@@ -442,6 +442,11 @@ URL resolve_directory(const URL& search, bool read_pkg_json) {
 URL Resolve(std::string specifier, const URL* base, bool read_pkg_json) {
   URL pure_url(specifier);
   if (!(pure_url.flags() & URL_FLAGS_FAILED)) {
+    // just check existence, without altering
+    auto check = check_file(pure_url, true);
+    if (check.failed) {
+      return URL("");
+    }
     return pure_url;
   }
   if (specifier.length() == 0) {
@@ -493,9 +498,8 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
 
   URL result = node::loader::Resolve(*specifier_utf, &url, true);
   if (result.flags() & URL_FLAGS_FAILED) {
-    std::string msg = "module ";
+    std::string msg = "Cannot find module ";
     msg += *specifier_utf;
-    msg += " not found";
     env->ThrowError(msg.c_str());
     return;
   }

test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs

@@ -0,0 +1,3 @@
+// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs
+/* eslint-disable required-modules */
+import './not-found.js';

test/es-module/test-esm-preserve-symlinks-not-found.mjs

@@ -0,0 +1,3 @@
+// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs
+/* eslint-disable required-modules */
+import './not-found';

test/fixtures/es-module-loaders/not-found-assert-loader.mjs

@@ -0,0 +1,22 @@
+import assert from 'assert';
+
+// a loader that asserts that the defaultResolve will throw "not found"
+// (skipping the top-level main of course)
+let mainLoad = true;
+export async function resolve (specifier, base, defaultResolve) {
+  if (mainLoad) {
+    mainLoad = false;
+    return defaultResolve(specifier, base);
+  }
+  try {
+    await defaultResolve(specifier, base);
+  }
+  catch (e) {
+    assert.equal(e.code, 'MODULE_NOT_FOUND');
+    return {
+      format: 'builtin',
+      url: 'fs'
+    };
+  }
+  assert.fail(`Module resolution for ${specifier} should be throw MODULE_NOT_FOUND`);
+}

test/parallel/test-module-main-fail.js

@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { execFileSync } = require('child_process');
+
+const entryPoints = ['iDoNotExist', 'iDoNotExist.js', 'iDoNotExist.mjs'];
+const flags = [[], ['--experimental-modules']];
+const node = process.argv[0];
+
+for (const args of flags) {
+  for (const entryPoint of entryPoints) {
+    try {
+      execFileSync(node, args.concat(entryPoint), { stdio: 'pipe' });
+    } catch (e) {
+      assert(e.toString().match(/Error: Cannot find module/));
+      continue;
+    }
+    assert.fail('Executing node with inexistent entry point should ' +
+                `fail. Entry point: ${entryPoint}, Flags: [${args}]`);
+  }
+}

test/parallel/test-module-main-preserve-symlinks-fail.js

@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { execFileSync } = require('child_process');
+
+const entryPoints = ['iDoNotExist', 'iDoNotExist.js', 'iDoNotExist.mjs'];
+const flags = [[], ['--experimental-modules', '--preserve-symlinks']];
+const node = process.argv[0];
+
+for (const args of flags) {
+  for (const entryPoint of entryPoints) {
+    try {
+      execFileSync(node, args.concat(entryPoint));
+    } catch (e) {
+      assert(e.toString().match(/Error: Cannot find module/));
+      continue;
+    }
+    assert.fail('Executing node with inexistent entry point should ' +
+                `fail. Entry point: ${entryPoint}, Flags: [${args}]`);
+  }
+}