Skip to content

Commit

Permalink
esm: better error message for unsupported URL
Browse files Browse the repository at this point in the history
The default ESM loader supports only file and data URLs.
This PR adds better error message for it.

PR-URL: #31129
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
Hakerh400 authored and MylesBorins committed Jan 12, 2020
1 parent b95bb38 commit c10ee5d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 10 deletions.
5 changes: 5 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1936,6 +1936,11 @@ An attempt was made to load a module with an unknown or unsupported format.
An invalid or unknown process signal was passed to an API expecting a valid
signal (such as [`subprocess.kill()`][]).

<a id="ERR_UNSUPPORTED_ESM_URL_SCHEME"></a>
### `ERR_UNSUPPORTED_ESM_URL_SCHEME`

`import` with URL schemes other than `file` and `data` is unsupported.

<a id="ERR_V8BREAKITERATOR"></a>
### `ERR_V8BREAKITERATOR`

Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,8 @@ E('ERR_UNKNOWN_FILE_EXTENSION',
TypeError);
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
E('ERR_UNSUPPORTED_ESM_URL_SCHEME', 'Only file and data URLs are supported ' +
'by the default ESM loader', Error);

E('ERR_V8BREAKITERATOR',
'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl',
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/modules/esm/default_resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const { resolve: moduleWrapResolve,
getPackageType } = internalBinding('module_wrap');
const { URL, pathToFileURL, fileURLToPath } = require('internal/url');
const { ERR_INPUT_TYPE_NOT_ALLOWED,
ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
ERR_UNKNOWN_FILE_EXTENSION,
ERR_UNSUPPORTED_ESM_URL_SCHEME } = require('internal/errors').codes;

const { SafeMap } = primordials;

Expand Down Expand Up @@ -50,8 +51,9 @@ if (experimentalJsonModules)
extensionFormatMap['.json'] = legacyExtensionFormatMap['.json'] = 'json';

function resolve(specifier, parentURL) {
let parsed;
try {
const parsed = new URL(specifier);
parsed = new URL(specifier);
if (parsed.protocol === 'data:') {
const [ , mime ] = /^([^/]+\/[^;,]+)(?:[^,]*?)(;base64)?,/.exec(parsed.pathname) || [ null, null, null ];
const format = ({
Expand All @@ -66,6 +68,8 @@ function resolve(specifier, parentURL) {
};
}
} catch {}
if (parsed && parsed.protocol !== 'file:' && parsed.protocol !== 'data:')
throw new ERR_UNSUPPORTED_ESM_URL_SCHEME();
if (NativeModule.canBeRequiredByUsers(specifier)) {
return {
url: specifier,
Expand Down
16 changes: 8 additions & 8 deletions test/es-module/test-esm-dynamic-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ function expectErrorProperty(result, propertyKey, value) {
}));
}

function expectMissingModuleError(result) {
expectErrorProperty(result, 'code', 'ERR_MODULE_NOT_FOUND');
function expectModuleError(result, err) {
expectErrorProperty(result, 'code', err);
}

function expectOkNamespace(result) {
Expand Down Expand Up @@ -55,10 +55,10 @@ function expectFsNamespace(result) {
expectFsNamespace(eval('import("fs")'));
expectFsNamespace(eval('import("fs")'));

expectMissingModuleError(import('./not-an-existing-module.mjs'));
// TODO(jkrems): Right now this doesn't hit a protocol error because the
// module resolution step already rejects it. These arguably should be
// protocol errors.
expectMissingModuleError(import('node:fs'));
expectMissingModuleError(import('http://example.com/foo.js'));
expectModuleError(import('./not-an-existing-module.mjs'),
'ERR_MODULE_NOT_FOUND');
expectModuleError(import('node:fs'),
'ERR_UNSUPPORTED_ESM_URL_SCHEME');
expectModuleError(import('http://example.com/foo.js'),
'ERR_UNSUPPORTED_ESM_URL_SCHEME');
})();

0 comments on commit c10ee5d

Please sign in to comment.