feedback7: asset downloads on a Node that cannot proxy fetch() warn and go direct instead of stopping (follow-up to PR #289)

- With HTTP(S)_PROXY set, Node 22.0–22.20 / 23 cannot route the built-in fetch through it; the port of #289 made
  fetch-assets (and so setup) fail there. Before 0.2.2 those downloads went direct, and a proxy variable left over
  from another tool must not stop a player's first start: restartForEnvProxy now logs the reason and returns false
  (go on in this process), guardDefaultFetch warns once and fetches. A Node that can proxy (≥ 22.21 / ≥ 24) still
  restarts once with NODE_USE_ENV_PROXY=1 and never fetches around the proxy.
- restartForEnvProxy takes an injectable `version` for the test. Tests: assets-env-proxy 9 / 9 (a new case for the
  old-Node path).

test/assets-env-proxy.test.js

@@ -55,13 +55,14 @@ test('startup switch: the variable, the flag, and NODE_OPTIONS; a late or lookal
   assert.equal(startedWithEnvProxy({}, []), false);
 });
 
-test('plan: unset proxy is off; a proxy restarts, is already on, or is refused', () => {
+test('plan: unset proxy is off; a proxy restarts, is already on, or (old Node) goes direct with a warning', () => {
   assert.equal(proxyConfigured({}), false);
   assert.equal(proxyConfigured({ HTTP_PROXY: '   ', NO_PROXY: 'localhost' }), false);
   assert.equal(envProxyPlan({}).action, 'off');
   assert.equal(envProxyPlan({ NO_PROXY: '127.0.0.1' }).action, 'off');
   assert.equal(envProxyPlan({ HTTP_PROXY: 'http://proxy.example:8080' }, '22.20.0', false).action, 'unsupported');
   assert.match(envProxyPlan({ HTTP_PROXY: 'http://proxy.example:8080' }, '23.11.0', true).reason, /Node 23\.11\.0/);
+  assert.match(envProxyPlan({ HTTP_PROXY: 'http://proxy.example:8080' }, '22.20.0', false).reason, /downloading directly instead/);
   assert.equal(envProxyPlan({ http_proxy: 'http://proxy.example:8080' }, '22.21.0', false).action, 'reexec');
   assert.match(envProxyPlan({ HTTPS_PROXY: 'http://proxy.example:8080' }, '24.0.0', false).reason, /Refusing to fetch directly/);
   // The flag on the object is not the startup snapshot: the third argument is.
@@ -147,12 +148,13 @@ test('restart decision: nothing to do, one spawn, or a hard stop — and the mar
       spawnImpl,
       error: (line) => logs.push(line),
     });
-    assert.equal(stopped, true);
     if (!nodeHonoursEnvProxy(process.versions.node)) {
+      assert.equal(stopped, false, 'an old Node goes on in this process (direct download)');
       assert.equal(spawned.length, 0);
       assert.match(logs.join('\n'), /does not route fetch/);
       return;
     }
+    assert.equal(stopped, true);
     assert.equal(spawned.length, 1);
     const [bin, args, opts] = spawned[0];
     assert.equal(bin, process.execPath);
@@ -303,3 +305,23 @@ test('built-in fetch tunnels through HTTP_PROXY only when NODE_USE_ENV_PROXY was
   assert.equal(bypass.body.body, 'from-target');
   assert.deepEqual(bypass.hits, ['TARGET /x']);
 });
+
+test('an old Node with HTTP(S)_PROXY set warns and downloads directly instead of stopping (0.2.2: a stray proxy variable must not break setup)', () => {
+  const prev = process.exitCode;
+  const spawned = [];
+  const logs = [];
+  try {
+    const goOn = restartForEnvProxy({
+      env: cleanEnv({ HTTPS_PROXY: 'http://proxy.example:8080' }),
+      version: '22.20.0',
+      spawnImpl: (...args) => { spawned.push(args); return new EventEmitter(); },
+      error: (line) => logs.push(line),
+    });
+    assert.equal(goOn, false, 'the caller continues in this process');
+    assert.equal(spawned.length, 0, 'no restart: this Node cannot proxy fetch()');
+    assert.notEqual(process.exitCode, 1, 'no failure exit code');
+    assert.match(logs.join('\n'), /Node 22\.20\.0 does not route fetch\(\) through it .*downloading directly instead/);
+  } finally {
+    process.exitCode = prev;
+  }
+});

tools/assets/downloader.mjs

@@ -9,7 +9,8 @@
 //   first (when enabled), then the original URL and jsDelivr fallback. The proxy gets one short attempt
 //   per file and shares a run-wide circuit breaker with the index/font fetchers.
 // - HTTP(S)_PROXY: the default fetch uses it only when this process was started
-//   with NODE_USE_ENV_PROXY=1 (Node >=22.21 or >=24). Otherwise it fails closed
+//   with NODE_USE_ENV_PROXY=1 (Node >=22.21 or >=24); on such a Node started
+//   without it, it fails closed; an older Node warns and connects directly
 //   (tools/assets/env-proxy.mjs). Pass fetchImpl to bypass that, as tests do.
 // - Idempotent: an existing file is kept when its size matches the ledger entry
 //   of a previous download or the expected byte count from research, or (when

tools/assets/env-proxy.mjs

@@ -5,13 +5,15 @@
 // fetch() honours this on Node >=22.21.0 and >=24.0.0. The CLI flag on the 24
 // line is >=24.5.0, so the restart sets the variable, which 24.0.0 already honours.
 // Assigning the variable later does nothing. Node 22.0–22.20 and 23 have no such
-// switch; engines stay ">=22", and a proxy there is an error rather than a direct
+// switch; engines stay ">=22": there the downloader warns and downloads directly,
-// download.
+// as every version did before 0.2.2 (a proxy variable left over from another tool
+// must not stop a player's setup).
 //
 // Nothing else reaches the built-in fetch: `node:undici` is not a public builtin,
 // and setGlobalDispatcher on the npm `undici` package does not affect it. The
-// fetch-assets script therefore restarts itself once. Any other caller of the
+// fetch-assets script therefore restarts itself once. On a Node that can proxy,
-// default fetch is rejected here while a proxy is configured.
+// any other caller of the default fetch is rejected while a proxy is configured
+// and the process was not started with the switch.
 
 import { spawn } from 'node:child_process';
 
@@ -72,8 +74,8 @@ export function envProxyPlan(env = process.env, version = process.versions.node,
   if (!nodeHonoursEnvProxy(version)) {
     return {
       action: 'unsupported',
-      reason: `HTTP(S)_PROXY is set, but Node ${version} does not route fetch() through it. `
+      reason: `HTTP(S)_PROXY is set, but Node ${version} does not route fetch() through it `
-        + 'Need Node >=22.21.0 or >=24.0.0, started with NODE_USE_ENV_PROXY=1. Refusing to fetch directly.',
+        + '(that needs Node >=22.21.0 or >=24.0.0, started with NODE_USE_ENV_PROXY=1); downloading directly instead.',
     };
   }
   if (startedWithFlag) return { action: 'already', reason: '' };
@@ -86,16 +88,19 @@ export function envProxyPlan(env = process.env, version = process.versions.node,
 
 /**
  * The built-in fetch, or the caller's own implementation unchanged.
- * When a proxy is configured and this process cannot use it, the default fetch
+ * When a proxy is configured and this Node could use it but was not started with
- * rejects instead of connecting to the origin.
+ * the switch, the default fetch rejects instead of connecting to the origin; on a
+ * Node that cannot proxy at all it warns once and connects directly.
  * @param {typeof fetch} [fetchImpl]
  * @returns {typeof fetch}
  */
 export function guardDefaultFetch(fetchImpl = globalThis.fetch) {
   if (fetchImpl !== globalThis.fetch) return fetchImpl;
+  let warned = false;
   return (input, init) => {
     const plan = envProxyPlan();
-    if (plan.action === 'reexec' || plan.action === 'unsupported') return Promise.reject(new Error(plan.reason));
+    if (plan.action === 'reexec') return Promise.reject(new Error(plan.reason));
+    if (plan.action === 'unsupported' && !warned) { warned = true; console.warn(`[assets] ${plan.reason}`); }
     return fetchImpl(input, init);
   };
 }
@@ -109,7 +114,8 @@ export function guardDefaultFetch(fetchImpl = globalThis.fetch) {
  * @param {string[]} [io.argv]
  * @param {NodeJS.ProcessEnv} [io.env]
  * @param {(line: string) => void} [io.error]
- * @returns {boolean} true when the caller must not continue (restarted, or refused)
+ * @param {string} [io.version] process.versions.node
+ * @returns {boolean} true when the caller must not continue (restarted, or refused); false to go on in this process
  */
 export function restartForEnvProxy({
   spawnImpl = spawn,
@@ -118,13 +124,13 @@ export function restartForEnvProxy({
   argv = process.argv,
   env = process.env,
   error = (line) => console.error(line),
+  version = process.versions.node,
 } = {}) {
-  const plan = envProxyPlan(env);
+  const plan = envProxyPlan(env, version);
   if (plan.action === 'off' || plan.action === 'already') return false;
   if (plan.action === 'unsupported') {
     error(`[assets] ${plan.reason}`);
-    process.exitCode = 1;
+    return false;
-    return true;
   }
   if (env[REEXEC_MARKER] === '1') {
     error('[assets] refused to restart again for HTTP(S)_PROXY');

tools/fetch-assets.mjs

@@ -19,7 +19,7 @@
 // cheap. Downloads use ~16 parallel connections, 3 retries per direct source,
 // a jsDelivr fallback and an opt-in GitHub proxy (one short attempt per URL).
 // HTTP(S)_PROXY is picked up by restarting once with NODE_USE_ENV_PROXY=1
-// (Node >=22.21 or >=24). An older Node fails instead of fetching directly.
+// (Node >=22.21 or >=24). An older Node warns and fetches directly, as before.
 // Spine atlases get `size:` (and `pma: true` for enemies); every skeleton is
 // parsed to resolve animation roles.
 //