From 233cf2af93b14dfd442a4d336381420e7c766982 Mon Sep 17 00:00:00 2001
From: guybedford <guybedford@gmail.com>
Date: Mon, 18 Jun 2018 13:58:11 +0200
Subject: [PATCH] error handling feedback

---
 lib/internal/errors.js                            |  4 ++--
 lib/internal/modules/esm/loader.js                | 11 +++++++++--
 test/es-module/test-esm-loader-invalid-url.mjs    | 15 +++++++--------
 .../es-module-loaders/loader-invalid-url.mjs      |  2 +-
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 47f9096d0a9616..ac87965421ad1a 100644
--- 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
index c53d16d3cf203d..a88b77f360df3e 100644
--- 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
index 440f8d8deff201..4ace7b183134a8 100644
--- 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
index c9b684151d2b19..12efbb5021a4bc 100644
--- 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'