From bc83382dcaf5a68a0a6f3fc21bb37a2aaec2fad8 Mon Sep 17 00:00:00 2001 From: guybedford Date: Mon, 18 Jun 2018 13:59:22 +0200 Subject: [PATCH 1/8] refactor common --- test/common/index.mjs | 186 +++++++++++++++++++++++++++--------------- 1 file changed, 119 insertions(+), 67 deletions(-) diff --git a/test/common/index.mjs b/test/common/index.mjs index b7ac556bca52c3..f73a9d9be5c08e 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -1,71 +1,123 @@ // Flags: --experimental-modules /* eslint-disable node-core/required-modules */ +import common from './index.js'; -import assert from 'assert'; +const { + PORT, + isMainThread, + isWindows, + isWOW64, + isAIX, + isLinuxPPCBE, + isSunOS, + isFreeBSD, + isOpenBSD, + isLinux, + isOSX, + isGlibc, + enoughTestMem, + enoughTestCpu, + rootDir, + buildType, + localIPv6Hosts, + opensslCli, + PIPE, + hasIPv6, + childShouldThrowAndAbort, + ddCommand, + spawnPwd, + spawnSyncPwd, + platformTimeout, + allowGlobals, + leakedGlobals, + mustCall, + mustCallAtLeast, + mustCallAsync, + hasMultiLocalhost, + fileExists, + skipIfEslintMissing, + canCreateSymLink, + getCallSite, + mustNotCall, + printSkipMessage, + skip, + ArrayStream, + nodeProcessAborted, + busyLoop, + isAlive, + noWarnCode, + expectWarning, + expectsError, + skipIfInspectorDisabled, + skipIf32Bits, + getArrayBufferViews, + getBufferSources, + crashOnUnhandledRejection, + getTTYfd, + runWithInvalidFD, + hijackStdout, + hijackStderr, + restoreStdout, + restoreStderr, + isCPPSymbolsNotMapped +} = common; -let knownGlobals = [ - Buffer, - clearImmediate, - clearInterval, - clearTimeout, - global, - process, - setImmediate, - setInterval, - setTimeout -]; - -if (process.env.NODE_TEST_KNOWN_GLOBALS) { - const knownFromEnv = process.env.NODE_TEST_KNOWN_GLOBALS.split(','); - allowGlobals(...knownFromEnv); -} - -export function allowGlobals(...whitelist) { - knownGlobals = knownGlobals.concat(whitelist); -} - -export function leakedGlobals() { - // Add possible expected globals - if (global.gc) { - knownGlobals.push(global.gc); - } - - if (global.DTRACE_HTTP_SERVER_RESPONSE) { - knownGlobals.push(DTRACE_HTTP_SERVER_RESPONSE); - knownGlobals.push(DTRACE_HTTP_SERVER_REQUEST); - knownGlobals.push(DTRACE_HTTP_CLIENT_RESPONSE); - knownGlobals.push(DTRACE_HTTP_CLIENT_REQUEST); - knownGlobals.push(DTRACE_NET_STREAM_END); - knownGlobals.push(DTRACE_NET_SERVER_CONNECTION); - } - - if (global.COUNTER_NET_SERVER_CONNECTION) { - knownGlobals.push(COUNTER_NET_SERVER_CONNECTION); - knownGlobals.push(COUNTER_NET_SERVER_CONNECTION_CLOSE); - knownGlobals.push(COUNTER_HTTP_SERVER_REQUEST); - knownGlobals.push(COUNTER_HTTP_SERVER_RESPONSE); - knownGlobals.push(COUNTER_HTTP_CLIENT_REQUEST); - knownGlobals.push(COUNTER_HTTP_CLIENT_RESPONSE); - } - - const leaked = []; - - for (const val in global) { - if (!knownGlobals.includes(global[val])) { - leaked.push(val); - } - } - - if (global.__coverage__) { - return leaked.filter((varname) => !/^(?:cov_|__cov)/.test(varname)); - } else { - return leaked; - } -} - -process.on('exit', function() { - const leaked = leakedGlobals(); - if (leaked.length > 0) { - assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`); - } -}); +export { + PORT, + isMainThread, + isWindows, + isWOW64, + isAIX, + isLinuxPPCBE, + isSunOS, + isFreeBSD, + isOpenBSD, + isLinux, + isOSX, + isGlibc, + enoughTestMem, + enoughTestCpu, + rootDir, + buildType, + localIPv6Hosts, + opensslCli, + PIPE, + hasIPv6, + childShouldThrowAndAbort, + ddCommand, + spawnPwd, + spawnSyncPwd, + platformTimeout, + allowGlobals, + leakedGlobals, + mustCall, + mustCallAtLeast, + mustCallAsync, + hasMultiLocalhost, + fileExists, + skipIfEslintMissing, + canCreateSymLink, + getCallSite, + mustNotCall, + printSkipMessage, + skip, + ArrayStream, + nodeProcessAborted, + busyLoop, + isAlive, + noWarnCode, + expectWarning, + expectsError, + skipIfInspectorDisabled, + skipIf32Bits, + getArrayBufferViews, + getBufferSources, + crashOnUnhandledRejection, + getTTYfd, + runWithInvalidFD, + hijackStdout, + hijackStderr, + restoreStdout, + restoreStderr, + isCPPSymbolsNotMapped +}; From cf1e32f832aec32d17dca8d09e59aa28a10e6c44 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 Jun 2018 15:40:31 +0200 Subject: [PATCH 2/8] esm: validate custom loader url --- lib/internal/modules/esm/loader.js | 10 ++++++++++ test/es-module/test-esm-loader-invalid-url.mjs | 10 ++++++++++ test/fixtures/es-module-loaders/loader-invalid-url.mjs | 8 ++++++++ 3 files changed, 28 insertions(+) create mode 100644 test/es-module/test-esm-loader-invalid-url.mjs create mode 100644 test/fixtures/es-module-loaders/loader-invalid-url.mjs diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index b3023fcd4f8e36..4ef2d12a4a5e90 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -3,9 +3,11 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_PROTOCOL, + ERR_INVALID_URL, ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, ERR_UNKNOWN_MODULE_FORMAT } = require('internal/errors').codes; +const { URL } = require('url'); const ModuleMap = require('internal/modules/esm/module_map'); const ModuleJob = require('internal/modules/esm/module_job'); const defaultResolve = require('internal/modules/esm/default_resolve'); @@ -64,6 +66,14 @@ class Loader { if (format === 'builtin') return { url: `node:${url}`, format }; + if (this._resolve !== defaultResolve) { + try { + new URL(url); + } catch (err) { + throw new ERR_INVALID_URL(url); + } + } + if (format !== 'dynamic' && !url.startsWith('file:')) throw new ERR_INVALID_PROTOCOL(url, 'file:'); diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs new file mode 100644 index 00000000000000..d1ef29cf034ff2 --- /dev/null +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -0,0 +1,10 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs +/* eslint-disable node-core/required-modules */ +import assert from 'assert'; + +import('../fixtures/es-modules/test-esm-ok.mjs') +.then(() => { + assert.fail(); +}, (err) => { + assert.strictEqual(err.code, 'ERR_INVALID_URL'); +}); diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs new file mode 100644 index 00000000000000..e64e66d49ad00b --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-invalid-url.mjs @@ -0,0 +1,8 @@ +export async function resolve(specifier, parentModuleURL, defaultResolve) { + if (parentModuleURL && specifier !== 'assert') + return { + url: specifier, + format: 'esm' + }; + return defaultResolve(specifier, parentModuleURL); +} From 65944622d3585c17831f0a34d0c0cd4959cd0c68 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 Jun 2018 17:03:55 +0200 Subject: [PATCH 3/8] braces nit --- test/fixtures/es-module-loaders/loader-invalid-url.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs index e64e66d49ad00b..c9b684151d2b19 100644 --- a/test/fixtures/es-module-loaders/loader-invalid-url.mjs +++ b/test/fixtures/es-module-loaders/loader-invalid-url.mjs @@ -1,8 +1,9 @@ export async function resolve(specifier, parentModuleURL, defaultResolve) { - if (parentModuleURL && specifier !== 'assert') + if (parentModuleURL && specifier !== 'assert') { return { url: specifier, format: 'esm' }; + } return defaultResolve(specifier, parentModuleURL); } From 1776edfacbd9efd7e9f3090cefd19c7d1e5e282f Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 Jun 2018 17:59:15 +0200 Subject: [PATCH 4/8] remove unnecessary try wrapper --- lib/internal/modules/esm/loader.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 4ef2d12a4a5e90..bcc15d28370e79 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -3,7 +3,6 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_PROTOCOL, - ERR_INVALID_URL, ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, ERR_UNKNOWN_MODULE_FORMAT } = require('internal/errors').codes; @@ -67,11 +66,8 @@ class Loader { return { url: `node:${url}`, format }; if (this._resolve !== defaultResolve) { - try { - new URL(url); - } catch (err) { - throw new ERR_INVALID_URL(url); - } + // throws ERR_INVALID_URL for resolve if not valid + new URL(url); } if (format !== 'dynamic' && !url.startsWith('file:')) From 888d186ea4dc5918d1ca75dcc5cb0f267221c112 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 17 Jun 2018 15:56:08 +0200 Subject: [PATCH 5/8] beautiful error messages --- doc/api/errors.md | 11 +++++++++ lib/internal/errors.js | 18 ++++++++++++-- lib/internal/modules/esm/loader.js | 24 ++++++++++++++----- .../es-module/test-esm-loader-invalid-url.mjs | 6 +++-- 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index c0773caf7c9bbc..487d558ec3f369 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1204,6 +1204,17 @@ An invalid `options.protocol` was passed. Both `breakEvalOnSigint` and `eval` options were set in the REPL config, which is not supported. + +### ERR_INVALID_RETURN_PROPERTY + +Thrown in case a function option does not return an expected property type. + + +### ERR_INVALID_RETURN_PROPERTY_STRING + +Thrown in case a function option does not return an expected string property +type. + ### ERR_INVALID_RETURN_VALUE diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 51bf07adf3d91b..47f9096d0a9616 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -681,6 +681,20 @@ E('ERR_INVALID_PROTOCOL', TypeError); E('ERR_INVALID_REPL_EVAL_CONFIG', 'Cannot specify both "breakEvalOnSigint" and "eval" for REPL', TypeError); +E('ERR_INVALID_RETURN_PROPERTY', (input, name, prop, value) => { + let type; + if (value && value.constructor && value.constructor.name) { + type = `instance of ${value.constructor.name}`; + } else { + type = `type ${typeof value}`; + } + return `Expected ${input} to be returned for the ${prop} from the ` + + `"${name}" function but got ${type}.`; +}, TypeError); +E('ERR_INVALID_RETURN_PROPERTY_STRING', (input, name, prop, value) => { + return `Expected a valid ${input} to be returned for the ${prop} from the ` + + `"${name}" function but got ${value}.`; +}, TypeError); E('ERR_INVALID_RETURN_VALUE', (input, name, value) => { let type; if (value && value.constructor && value.constructor.name) { @@ -688,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 bcc15d28370e79..c53d16d3cf203d 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -2,7 +2,8 @@ const { ERR_INVALID_ARG_TYPE, - ERR_INVALID_PROTOCOL, + ERR_INVALID_RETURN_PROPERTY, + ERR_INVALID_RETURN_PROPERTY_STRING, ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, ERR_UNKNOWN_MODULE_FORMAT } = require('internal/errors').codes; @@ -57,21 +58,32 @@ class Loader { await this._resolve(specifier, parentURL, defaultResolve); if (typeof url !== 'string') - throw new ERR_INVALID_ARG_TYPE('url', 'string', url); + throw new ERR_INVALID_RETURN_PROPERTY( + 'string', 'loader resolve', 'url', url + ); if (typeof format !== 'string') - throw new ERR_INVALID_ARG_TYPE('format', 'string', format); + throw new ERR_INVALID_RETURN_PROPERTY_STRING( + 'string', 'loader resolve', 'format', format + ); if (format === 'builtin') return { url: `node:${url}`, format }; if (this._resolve !== defaultResolve) { - // throws ERR_INVALID_URL for resolve if not valid - new URL(url); + try { + new URL(url); + } catch (e) { + throw new ERR_INVALID_RETURN_PROPERTY_STRING( + 'url', 'loader resolve', 'url', url + ); + } } if (format !== 'dynamic' && !url.startsWith('file:')) - throw new ERR_INVALID_PROTOCOL(url, 'file:'); + throw new ERR_INVALID_RETURN_PROPERTY_STRING( + 'file: url', 'loader resolve', 'url', url + ); return { url, format }; } diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs index d1ef29cf034ff2..440f8d8deff201 100644 --- a/test/es-module/test-esm-loader-invalid-url.mjs +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -1,10 +1,12 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs /* eslint-disable node-core/required-modules */ 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_URL'); -}); + assert.strictEqual(err.code, 'ERR_INVALID_RETURN_PROPERTY_STRING'); +}) +.then(common.mustCall()); From 233cf2af93b14dfd442a4d336381420e7c766982 Mon Sep 17 00:00:00 2001 From: guybedford Date: Mon, 18 Jun 2018 13:58:11 +0200 Subject: [PATCH 6/8] 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' From d43436809aa136461aa62b823e7f1020f4288107 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 18 Jun 2018 23:47:51 +0200 Subject: [PATCH 7/8] test case refinements --- doc/api/errors.md | 14 ++++++++------ lib/internal/errors.js | 12 ++++++------ lib/internal/modules/esm/loader.js | 12 ++++++------ test/es-module/test-esm-loader-invalid-url.mjs | 7 ++++--- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 487d558ec3f369..4d871c0c1c188e 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1207,18 +1207,20 @@ is not supported. ### ERR_INVALID_RETURN_PROPERTY -Thrown in case a function option does not return an expected property type. +Thrown in case a function option does not provide a valid value for one of its +returned object properties on execution. - -### ERR_INVALID_RETURN_PROPERTY_STRING + +### ERR_INVALID_RETURN_PROPERTY_VALUE -Thrown in case a function option does not return an expected string property -type. +Thrown in case a function option does not provide an expected value +type for one of its returned object properties on execution. ### ERR_INVALID_RETURN_VALUE -Thrown in case a function option does not return an expected value on execution. +Thrown in case a function option does not return an expected value +type on execution. For example when a function is expected to return a promise. diff --git a/lib/internal/errors.js b/lib/internal/errors.js index ac87965421ad1a..507a42962c45b5 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -682,18 +682,18 @@ E('ERR_INVALID_PROTOCOL', E('ERR_INVALID_REPL_EVAL_CONFIG', 'Cannot specify both "breakEvalOnSigint" and "eval" for REPL', TypeError); E('ERR_INVALID_RETURN_PROPERTY', (input, name, prop, value) => { + return `Expected a valid ${input} to be returned for the "${prop}" from the` + + ` "${name}" function but got ${value}.`; +}, TypeError); +E('ERR_INVALID_RETURN_PROPERTY_VALUE', (input, name, prop, value) => { let type; if (value && value.constructor && value.constructor.name) { type = `instance of ${value.constructor.name}`; } else { type = `type ${typeof value}`; } - return `Expected ${input} to be returned for the ${prop} from the ` + - `"${name}" function but got ${type}.`; -}, TypeError); -E('ERR_INVALID_RETURN_PROPERTY_STRING', (input, name, prop, value) => { - return `Expected a valid ${input} to be returned for the ${prop} from the ` + - `"${name}" function but got ${value}.`; + return `Expected ${input} to be returned for the "${prop}" from the` + + ` "${name}" function but got ${type}.`; }, TypeError); E('ERR_INVALID_RETURN_VALUE', (input, name, value) => { let type; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index a88b77f360df3e..17a9686f736060 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -3,7 +3,7 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_RETURN_PROPERTY, - ERR_INVALID_RETURN_PROPERTY_STRING, + ERR_INVALID_RETURN_PROPERTY_VALUE, ERR_INVALID_RETURN_VALUE, ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, ERR_UNKNOWN_MODULE_FORMAT @@ -65,13 +65,13 @@ class Loader { const { url, format } = resolved; if (typeof url !== 'string') - throw new ERR_INVALID_RETURN_PROPERTY( + throw new ERR_INVALID_RETURN_PROPERTY_VALUE( 'string', 'loader resolve', 'url', url ); if (typeof format !== 'string') - throw new ERR_INVALID_RETURN_PROPERTY_STRING( - 'string', 'loader resolve', 'format', format + throw new ERR_INVALID_RETURN_PROPERTY( + 'module format', 'loader resolve', 'format', format ); if (format === 'builtin') @@ -81,14 +81,14 @@ class Loader { try { new URL(url); } catch (e) { - throw new ERR_INVALID_RETURN_PROPERTY_STRING( + throw new ERR_INVALID_RETURN_PROPERTY( 'url', 'loader resolve', 'url', url ); } } if (format !== 'dynamic' && !url.startsWith('file:')) - throw new ERR_INVALID_RETURN_PROPERTY_STRING( + throw new ERR_INVALID_RETURN_PROPERTY( 'file: url', 'loader resolve', 'url', url ); diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs index 4ace7b183134a8..43971a2e6e3b71 100644 --- a/test/es-module/test-esm-loader-invalid-url.mjs +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -4,8 +4,9 @@ import assert from 'assert'; import('../fixtures/es-modules/test-esm-ok.mjs') .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.' + code: 'ERR_INVALID_RETURN_PROPERTY', + 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()); From a235f086790dd5b8fe01c82ed0b40e432581e356 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 26 Jun 2018 12:25:30 +0200 Subject: [PATCH 8/8] format case test and fix --- lib/internal/modules/esm/loader.js | 4 ++-- test/es-module/test-esm-loader-invalid-format.mjs | 11 +++++++++++ .../es-module-loaders/loader-invalid-format.mjs | 8 ++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 test/es-module/test-esm-loader-invalid-format.mjs create mode 100644 test/fixtures/es-module-loaders/loader-invalid-format.mjs diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 17a9686f736060..2fa10c7eab9b44 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -70,8 +70,8 @@ class Loader { ); if (typeof format !== 'string') - throw new ERR_INVALID_RETURN_PROPERTY( - 'module format', 'loader resolve', 'format', format + throw new ERR_INVALID_RETURN_PROPERTY_VALUE( + 'string', 'loader resolve', 'format', format ); if (format === 'builtin') diff --git a/test/es-module/test-esm-loader-invalid-format.mjs b/test/es-module/test-esm-loader-invalid-format.mjs new file mode 100644 index 00000000000000..f8714d4aa11d59 --- /dev/null +++ b/test/es-module/test-esm-loader-invalid-format.mjs @@ -0,0 +1,11 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs +import { expectsError, mustCall } from '../common'; +import assert from 'assert'; + +import('../fixtures/es-modules/test-esm-ok.mjs') +.then(assert.fail, expectsError({ + code: 'ERR_INVALID_RETURN_PROPERTY_VALUE', + message: 'Expected string to be returned for the "format" from the ' + + '"loader resolve" function but got type undefined.' +})) +.then(mustCall()); diff --git a/test/fixtures/es-module-loaders/loader-invalid-format.mjs b/test/fixtures/es-module-loaders/loader-invalid-format.mjs new file mode 100644 index 00000000000000..17a0dcd04daad9 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-invalid-format.mjs @@ -0,0 +1,8 @@ +export async function resolve(specifier, parentModuleURL, defaultResolve) { + if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { + return { + url: 'file:///asdf' + }; + } + return defaultResolve(specifier, parentModuleURL); +}