module: prefer async/await in https imports

lib/internal/modules/esm/fetch_module.js

@@ -1,26 +1,24 @@
 'use strict';
 const {
- ArrayPrototypePush,
- Promise,
+ ObjectPrototypeHasOwnProperty,
  PromisePrototypeThen,
- PromiseResolve,
  SafeMap,
  StringPrototypeEndsWith,
  StringPrototypeSlice,
  StringPrototypeStartsWith,
 } = primordials;
 const {
- Buffer: {
- concat: BufferConcat
- }
+ Buffer: { concat: BufferConcat },
 } = require('buffer');
 const {
  ERR_NETWORK_IMPORT_DISALLOWED,
  ERR_NETWORK_IMPORT_BAD_RESPONSE,
+ ERR_MODULE_NOT_FOUND,
 } = require('internal/errors').codes;
 const { URL } = require('internal/url');
 const net = require('net');
-
+const { once } = require('events');
+const { compose } = require('stream');
 /**
  * @typedef CacheEntry
  * @property {Promise<string> | string} resolvedHREF
@@ -32,6 +30,9 @@ const net = require('net');
  * Only for GET requests, other requests would need new Map
  * HTTP cache semantics keep diff caches
  *
+ * It caches either the promise or the cache entry since import.meta.url needs
+ * the value synchronously for the response location after all redirects.
+ *
  * Maps HREF to pending cache entry
  * @type {Map<string, Promise<CacheEntry> | CacheEntry>}
  */
@@ -47,23 +48,23 @@ let HTTPSAgent;
 function HTTPSGet(url, opts) {
  const https = require('https'); // [1]
  HTTPSAgent ??= new https.Agent({ // [2]
- keepAlive: true
+ keepAlive: true,
  });
  return https.get(url, {
  agent: HTTPSAgent,
- ...opts
+ ...opts,
  });
 }
 
 let HTTPAgent;
 function HTTPGet(url, opts) {
  const http = require('http'); // [1]
  HTTPAgent ??= new http.Agent({ // [2]
- keepAlive: true
+ keepAlive: true,
  });
  return http.get(url, {
  agent: HTTPAgent,
- ...opts
+ ...opts,
  });
 }
 
@@ -98,118 +99,79 @@ function fetchWithRedirects(parsed) {
  return existing;
  }
  const handler = parsed.protocol === 'http:' ? HTTPGet : HTTPSGet;
- const result = new Promise((fulfill, reject) => {
+ const result = (async () => {
  const req = handler(parsed, {
- headers: {
- Accept: '*/*'
- }
- })
- .on('error', reject)
- .on('response', (res) => {
- function dispose() {
- req.destroy();
- res.destroy();
- }
- if (res.statusCode >= 300 && res.statusCode <= 303) {
- if (res.headers.location) {
- dispose();
- try {
- const location = new URL(res.headers.location, parsed);
- if (location.protocol !== 'http:' &&
- location.protocol !== 'https:') {
- reject(new ERR_NETWORK_IMPORT_DISALLOWED(
- res.headers.location,
- parsed.href,
- 'cannot redirect to non-network location'));
- return;
- }
- return PromisePrototypeThen(
- PromiseResolve(fetchWithRedirects(location)),
- (entry) => {
- cacheForGET.set(parsed.href, entry);
- fulfill(entry);
- });
- } catch (e) {
- dispose();
- reject(e);
- }
+ headers: { Accept: '*/*' },
+ });
+ // Note that `once` is used here to handle `error` and that it hits the
+ // `finally` on network error/timeout.
+ const { 0: res } = await once(req, 'response');
+ try {
+ const isRedirect = res.statusCode >= 300 && res.statusCode <= 303;
+ const hasLocation = ObjectPrototypeHasOwnProperty(res.headers, 'location');
+ if (isRedirect && hasLocation) {
+ const location = new URL(res.headers.location, parsed);
+ if (location.protocol !== 'http:' && location.protocol !== 'https:') {
+ throw new ERR_NETWORK_IMPORT_DISALLOWED(
+ res.headers.location,
+ parsed.href,
+ 'cannot redirect to non-network location'
+ );
  }
+ const entry = await fetchWithRedirects(location);
+ cacheForGET.set(parsed.href, entry);
+ return entry;
+ }
+ if (res.statusCode === 404) {
+ const err = new ERR_MODULE_NOT_FOUND(parsed.href, null);
+ err.message = `Cannot find module '${parsed.href}', HTTP 404`;
+ throw err;
  }
  if (res.statusCode > 303 || res.statusCode < 200) {
- dispose();
- reject(
- new ERR_NETWORK_IMPORT_BAD_RESPONSE(
- parsed.href,
- 'HTTP response returned status code of ' + res.statusCode));
- return;
+ throw new ERR_NETWORK_IMPORT_DISALLOWED(
+ res.headers.location,
+ parsed.href,
+ 'cannot redirect to non-network location');
  }
  const { headers } = res;
  const contentType = headers['content-type'];
  if (!contentType) {
- dispose();
- reject(new ERR_NETWORK_IMPORT_BAD_RESPONSE(
+ throw new ERR_NETWORK_IMPORT_BAD_RESPONSE(
  parsed.href,
-'the \'Content-Type\' header is required'));
-return;
+"the 'Content-Type' header is required"
+);
  }
  /**
  * @type {CacheEntry}
  */
  const entry = {
  resolvedHREF: parsed.href,
  headers: {
- 'content-type': res.headers['content-type']
+ 'content-type': res.headers['content-type'],
  },
- body: new Promise((f, r) => {
- const buffers = [];
- let size = 0;
+ body: (async () => {
  let bodyStream = res;
- let onError;
  if (res.headers['content-encoding'] === 'br') {
- bodyStream = createBrotliDecompress();
- onError = function onError(error) {
- bodyStream.close();
- dispose();
- reject(error);
- r(error);
- };
- res.on('error', onError);
- res.pipe(bodyStream);
- } else if (res.headers['content-encoding'] === 'gzip' ||
- res.headers['content-encoding'] === 'deflate') {
- bodyStream = createUnzip();
- onError = function onError(error) {
- bodyStream.close();
- dispose();
- reject(error);
- r(error);
- };
- res.on('error', onError);
- res.pipe(bodyStream);
- } else {
- onError = function onError(error) {
- dispose();
- reject(error);
- r(error);
- };
+ bodyStream = compose(res, createBrotliDecompress());
+ } else if (
+ res.headers['content-encoding'] === 'gzip' ||
+ res.headers['content-encoding'] === 'deflate'
+ ) {
+ bodyStream = compose(res, createUnzip());
  }
- bodyStream.on('error', onError);
- bodyStream.on('data', (d) => {
- ArrayPrototypePush(buffers, d);
- size += d.length;
- });
- bodyStream.on('end', () => {
- const body = entry.body = /** @type {Buffer} */(
- BufferConcat(buffers, size)
- );
- f(body);
- });
- }),
+ const buffers = await bodyStream.toArray();
+ const body = BufferConcat(buffers);
+ entry.body = body;
+ return body;
+ })(),
  };
  cacheForGET.set(parsed.href, entry);
- fulfill(entry);
- });
- });
+ await entry.body;
+ return entry;
+ } finally {
+ req.destroy();
+ }
+ })();
  cacheForGET.set(parsed.href, result);
  return result;
 }
@@ -226,8 +188,10 @@ allowList.addRange('127.0.0.1', '127.255.255.255');
  */
 async function isLocalAddress(hostname) {
  try {
- if (StringPrototypeStartsWith(hostname, '[') &&
- StringPrototypeEndsWith(hostname, ']')) {
+ if (
+ StringPrototypeStartsWith(hostname, '[') &&
+ StringPrototypeEndsWith(hostname, ']')
+ ) {
  hostname = StringPrototypeSlice(hostname, 1, -1);
  }
  const addr = await dnsLookup(hostname, { verbatim: true });
@@ -275,5 +239,5 @@ function fetchModule(parsed, { parentURL }) {
 }
 
 module.exports = {
- fetchModule: fetchModule
+ fetchModule: fetchModule,
 };

test/es-module/test-http-imports.mjs

@@ -64,6 +64,11 @@ for (const { protocol, createServer } of [
  const server = createServer(function(_req, res) {
  const url = new URL(_req.url, host);
  const redirect = url.searchParams.get('redirect');
+ if (url.pathname === '/not-found') {
+ res.writeHead(404);
+ res.end();
+ return;
+ }
  if (redirect) {
  const { status, location } = JSON.parse(redirect);
  res.writeHead(status, {
@@ -165,6 +170,12 @@ for (const { protocol, createServer } of [
  import(unsupportedMIME.href),
  { code: 'ERR_UNKNOWN_MODULE_FORMAT' }
  );
+ const notFound = new URL(url.href);
+ notFound.pathname = '/not-found';
+ await assert.rejects(
+ import(notFound.href),
+ { code: 'ERR_MODULE_NOT_FOUND' },
+ );
 
  server.close();
  }