error handling feedback

nodejs · Jun 18, 2018 · 233cf2a · 233cf2a
1 parent 888d186
commit 233cf2a
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 13 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -702,8 +702,8 @@ E('ERR_INVALID_RETURN_VALUE', (input, name, value) => {
   } else {
     type = `type ${typeof value}`;
   }
-  return `Expected ${input} to be returned from the ` +
-         `"${name}" function but got ${type}.`;
+  return `Expected ${input} to be returned from the "${name}"` +
+         ` function but got ${type}.`;
 }, TypeError);
 E('ERR_INVALID_SYNC_FORK_INPUT',
   'Asynchronous forks do not support Buffer, Uint8Array or string input: %s',

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -4,6 +4,7 @@ const {
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_RETURN_PROPERTY,
   ERR_INVALID_RETURN_PROPERTY_STRING,
+  ERR_INVALID_RETURN_VALUE,
   ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK,
   ERR_UNKNOWN_MODULE_FORMAT
 } = require('internal/errors').codes;
@@ -54,8 +55,14 @@ class Loader {
     if (!isMain && typeof parentURL !== 'string')
       throw new ERR_INVALID_ARG_TYPE('parentURL', 'string', parentURL);
 
-    const { url, format } =
-      await this._resolve(specifier, parentURL, defaultResolve);
+    const resolved = await this._resolve(specifier, parentURL, defaultResolve);
+
+    if (typeof resolved !== 'object')
+      throw new ERR_INVALID_RETURN_VALUE(
+        'object', 'loader resolve', resolved
+      );
+
+    const { url, format } = resolved;
 
     if (typeof url !== 'string')
       throw new ERR_INVALID_RETURN_PROPERTY(

diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs
@@ -1,12 +1,11 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs
-/* eslint-disable node-core/required-modules */
+import { expectsError, mustCall } from '../common';
 import assert from 'assert';
-import common from '../common';
 
 import('../fixtures/es-modules/test-esm-ok.mjs')
-.then(() => {
-  assert.fail();
-}, (err) => {
-  assert.strictEqual(err.code, 'ERR_INVALID_RETURN_PROPERTY_STRING');
-})
-.then(common.mustCall());
+.then(assert.fail, expectsError({
+  code: 'ERR_INVALID_RETURN_PROPERTY_STRING',
+  message: 'Expected a valid url to be returned for the url from the "loader ' +
+           'resolve" function but got ../fixtures/es-modules/test-esm-ok.mjs.'
+}))
+.then(mustCall());
diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs
@@ -1,5 +1,5 @@
 export async function resolve(specifier, parentModuleURL, defaultResolve) {
-  if (parentModuleURL && specifier !== 'assert') {
+  if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') {
     return {
       url: specifier,
       format: 'esm'