module: throw an error for invalid package.json main entries

We currently ignore invalid `main` entries in package.json files.
This does not seem to be very user friendly as it's certainly an
error if the `main` entry is not a valid file name. So instead of
trying to resolve the file otherwise, throw an error immediately to
improve the user experience.
To keep it backwards compatible `index.js` files in the same directory
as the `package.json` will continue to be resolved instead but that
behavior is now deprecated.

PR-URL: https://github.com/nodejs/node/pull/26823
Fixes: https://github.com/nodejs/node/issues/26588
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

6 files changed, 44 insertions, 14 deletions

diff --git a/test/common/index.js b/test/common/index.js
index 94b90ac27b..d6a24dd944 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -494,7 +494,11 @@ function _expectWarning(name, expected, code) {
   return mustCall((warning) => {
     const [ message, code ] = expected.shift();
     assert.strictEqual(warning.name, name);
-    assert.strictEqual(warning.message, message);
+    if (typeof message === 'string') {
+      assert.strictEqual(warning.message, message);
+    } else {
+      assert(message.test(warning.message));
+    }
     assert.strictEqual(warning.code, code);
   }, expected.length);
 }
diff --git a/test/fixtures/packages/missing-main-no-index/package.json b/test/fixtures/packages/missing-main-no-index/package.json
new file mode 100644
index 0000000000..feb846703f
--- /dev/null
+++ b/test/fixtures/packages/missing-main-no-index/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "missingmain",
+  "main": "doesnotexist.js"
+}
diff --git a/test/fixtures/packages/missing-main-no-index/stray.js b/test/fixtures/packages/missing-main-no-index/stray.js
new file mode 100644
index 0000000000..3960ed1a9d
--- /dev/null
+++ b/test/fixtures/packages/missing-main-no-index/stray.js
@@ -0,0 +1,2 @@
+// This file should not be loaded.
+throw new Error('Failed');
diff --git a/test/parallel/test-module-loading-deprecated.js b/test/parallel/test-module-loading-deprecated.js
new file mode 100644
index 0000000000..3aa81eea6e
--- /dev/null
+++ b/test/parallel/test-module-loading-deprecated.js
@@ -0,0 +1,12 @@
+// Flags: --pending-deprecation
+
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+common.expectWarning('DeprecationWarning', {
+  DEP0128: /^Invalid 'main' field in '.+main[/\\]package\.json' of 'doesnotexist\.js'\..+module author/
+});
+
+assert.strictEqual(require('../fixtures/packages/missing-main').ok, 'ok');
diff --git a/test/parallel/test-require-exceptions.js b/test/parallel/test-require-exceptions.js
index 132bffed0f..70d6b8334a 100644
--- a/test/parallel/test-require-exceptions.js
+++ b/test/parallel/test-require-exceptions.js
@@ -26,31 +26,28 @@ const fs = require('fs');
 const fixtures = require('../common/fixtures');
 
 // A module with an error in it should throw
-assert.throws(function() {
+assert.throws(() => {
   require(fixtures.path('/throws_error'));
 }, /^Error: blah$/);
 
 // Requiring the same module again should throw as well
-assert.throws(function() {
+assert.throws(() => {
   require(fixtures.path('/throws_error'));
 }, /^Error: blah$/);
 
 // Requiring a module that does not exist should throw an
 // error with its `code` set to MODULE_NOT_FOUND
-assertModuleNotFound('/DOES_NOT_EXIST');
+assert.throws(
+  () => require('/DOES_NOT_EXIST'),
+  { code: 'MODULE_NOT_FOUND' }
+);
 
 assertExists('/module-require/not-found/trailingSlash.js');
 assertExists('/module-require/not-found/node_modules/module1/package.json');
-assertModuleNotFound('/module-require/not-found/trailingSlash');
-
-function assertModuleNotFound(path) {
-  assert.throws(function() {
-    require(fixtures.path(path));
-  }, function(e) {
-    assert.strictEqual(e.code, 'MODULE_NOT_FOUND');
-    return true;
-  });
-}
+assert.throws(
+  () => require('/module-require/not-found/trailingSlash'),
+  { code: 'MODULE_NOT_FOUND' }
+);
 
 function assertExists(fixture) {
   assert(fs.existsSync(fixtures.path(fixture)));
diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js
index 0269445c76..630830f5dd 100644
--- a/test/sequential/test-module-loading.js
+++ b/test/sequential/test-module-loading.js
@@ -29,6 +29,8 @@ const path = require('path');
 
 const backslash = /\\/g;
 
+process.on('warning', common.mustNotCall());
+
 console.error('load test-module-loading.js');
 
 assert.strictEqual(require.main.id, '.');
@@ -105,6 +107,15 @@ assert.strictEqual(require('../fixtures/packages/index').ok, 'ok');
 assert.strictEqual(require('../fixtures/packages/main').ok, 'ok');
 assert.strictEqual(require('../fixtures/packages/main-index').ok, 'ok');
 assert.strictEqual(require('../fixtures/packages/missing-main').ok, 'ok');
+assert.throws(
+  () => require('../fixtures/packages/missing-main-no-index'),
+  {
+    code: 'MODULE_NOT_FOUND',
+    message: /packages[/\\]missing-main-no-index[/\\]doesnotexist\.js'\. Please.+package\.json.+valid "main"/,
+    path: /fixtures[/\\]packages[/\\]missing-main-no-index[/\\]package\.json/,
+    requestPath: /^\.\.[/\\]fixtures[/\\]packages[/\\]missing-main-no-index$/
+  }
+);
 
 assert.throws(
   function() { require('../fixtures/packages/unparseable'); },