From cddbc167495f8cee68028f05b682b7c5817f5a71 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Tue, 28 Jul 2026 22:10:28 +0000 Subject: [PATCH 1/2] Clarify installed vs built-in undici terminology. The code used "bundled" for the npm-installed undici and "runtime"/"native" for the one built into node, which reads backwards and made `bundledMajor === runtimeMajor` hard to follow. Standardize on "installed" and "built-in" throughout. Co-Authored-By: Claude Opus 5 (1M context) --- lib/agentCompatibility.js | 60 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/lib/agentCompatibility.js b/lib/agentCompatibility.js index a2067df..d46055f 100644 --- a/lib/agentCompatibility.js +++ b/lib/agentCompatibility.js @@ -13,32 +13,31 @@ const AGENT_CACHE = new WeakMap(); const [major, minor] = versions.node.split('.').map(v => parseInt(v, 10)); const canConvert = (major > 18) || (major === 18 && minor >= 2); -// A dispatcher built from the bundled undici's `Agent` shares a handler -// contract with the runtime's `fetch` only when their undici majors match. -// This package installs undici 6; node's own bundled undici major varies by -// release line and does not necessarily match that -- today node 22 bundles -// undici 6 (matches), while node 24 bundles undici 7 and node 26 bundles -// undici 8 (both mismatch, so both already take the fallback path below, -// not just node 26). The contract that breaks (the dispatcher handler's -// `onError`) changed across those majors, so a mismatched pairing rejects -// the bundled v6 dispatcher with "invalid onError method". When they match -// we hand the dispatcher to `ky`, which forwards it to the runtime fetch (ky +// A dispatcher built from the installed undici's `Agent` shares a handler +// contract with node's built-in `fetch` only when their undici majors match. +// This package installs undici 6; the undici built into node varies by +// release line and does not necessarily match that -- today node 22 has +// undici 6 built in (matches), while node 24 has 7 and node 26 has 8 (both +// mismatch, so both already take the fallback path below, not just node 26). +// The contract that breaks (the dispatcher handler's `onError`) changed +// across those majors, so a mismatched pairing rejects the installed v6 +// dispatcher with "invalid onError method". When they match we hand the +// dispatcher to `ky`, which forwards it to the built-in fetch (ky // deliberately keeps `dispatcher` out of its request-option registry so it -// reaches fetch). When they differ we call the bundled undici's own fetch, -// which cannot consume the runtime's `Request` class directly, so it is -// rebuilt as the bundled undici's own `Request` first (see `createFetch` -// below). This skew only exists because node does not -// expose its built-in undici (`node:undici`); see -// digitalbazaar/http-client#43. +// reaches fetch). When they differ we call the installed undici's own fetch, +// which cannot consume node's built-in `Request` class directly, so it is +// rebuilt as the installed undici's own `Request` first (see `createFetch` +// below). This skew only exists because node does not expose its built-in +// undici (`node:undici`); see digitalbazaar/http-client#43. // The version read is guarded: if a future undici hides `package.json` behind // an `exports` map, or `process.versions.undici` is absent, default to the -// bundled undici's own fetch (the always-safe path) rather than throwing at +// installed undici's own fetch (the always-safe path) rather than throwing at // module load and breaking `import` for every consumer. -const nativeFetchCompatible = (() => { +const builtinFetchCompatible = (() => { try { - const bundledMajor = parseInt(undiciPkg.version, 10); - const runtimeMajor = parseInt(versions.undici, 10); - return runtimeMajor === bundledMajor; + const installedMajor = parseInt(undiciPkg.version, 10); + const builtinMajor = parseInt(versions.undici, 10); + return builtinMajor === installedMajor; } catch{ return false; } @@ -73,14 +72,14 @@ export function convertAgent(options) { delete rest.agent; delete rest.httpsAgent; - // compatible runtime: let `ky` forward the dispatcher to the native `fetch`, - // which consumes the runtime `Request` natively — no wrapper, native perf - if(nativeFetchCompatible) { + // compatible: let `ky` forward the dispatcher to node's built-in `fetch`, + // which consumes its own `Request` natively — no wrapper, native perf + if(builtinFetchCompatible) { return {...rest, dispatcher}; } - // incompatible runtime `fetch` that rejects this dispatcher, so route - // through the bundled undici's own fetch via an override + // incompatible built-in `fetch` that rejects this dispatcher, so route + // through the installed undici's own fetch via an override let fetch = AGENT_CACHE.get(dispatcher); if(!fetch) { fetch = createFetch(dispatcher); @@ -90,10 +89,11 @@ export function convertAgent(options) { return {...rest, fetch}; } -// create fetch override uses custom `dispatcher`; on an incompatible runtime -// `ky`'s runtime `Request` cannot be consumed by the bundled undici's fetch -// directly, so it is rebuilt as the bundled undici's own `Request` here. -// Passing the runtime `Request` as undici's `Request` *init* (its second +// create fetch override uses custom `dispatcher`; when incompatible, the +// built-in `Request` that `ky` creates cannot be consumed by the installed +// undici's fetch directly, so it is rebuilt as the installed undici's own +// `Request` here. +// Passing the built-in `Request` as undici's `Request` *init* (its second // constructor argument) works because undici's own `Request` constructor // performs its own `RequestInit` dictionary conversion -- it reads exactly // the fields its own implementation understands directly off the object it's From 84a019c43668c6ec4fa7c41d262897c81429d407 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Tue, 28 Jul 2026 23:19:24 +0000 Subject: [PATCH 2/2] Split the undici compatibility comment by concern. The comment above `builtinFetchCompatible` had grown to explain the background, the constant, both branches, and the guarded version reads all at once. Hoist the background to a module-level note, leave the constant with only its own meaning, and move the note about `ky` forwarding `dispatcher` to the branch that relies on it. Drop the `Request` rebuild explanation, which `createFetch` already documents. Co-Authored-By: Claude Opus 5 (1M context) --- lib/agentCompatibility.js | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/lib/agentCompatibility.js b/lib/agentCompatibility.js index d46055f..faf6f31 100644 --- a/lib/agentCompatibility.js +++ b/lib/agentCompatibility.js @@ -5,6 +5,16 @@ import {Agent, fetch as undiciFetch, Request as UndiciRequest} from 'undici'; import undiciPkg from 'undici/package.json' with {type: 'json'}; import {versions} from 'node:process'; +// Background: node has its own copy of undici built in but does not expose +// it (there is no `node:undici`), so this package installs its own. A +// dispatcher only works with the undici that created it -- the handler +// contract changed across majors, so handing an installed v6 dispatcher to +// a built-in v7 or v8 `fetch` fails with "invalid onError method". Which +// major node has built in varies by release line (node 22 has 6, node 24 +// has 7, node 26 has 8), so no single installed version matches every +// supported runtime -- with undici 6 installed, both node 24 and node 26 +// take the fallback path below. See digitalbazaar/http-client#43. + // as long as an agent has a reference to it, its associated dispatcher will // be kept in this cache for reuse const AGENT_CACHE = new WeakMap(); @@ -13,25 +23,11 @@ const AGENT_CACHE = new WeakMap(); const [major, minor] = versions.node.split('.').map(v => parseInt(v, 10)); const canConvert = (major > 18) || (major === 18 && minor >= 2); -// A dispatcher built from the installed undici's `Agent` shares a handler -// contract with node's built-in `fetch` only when their undici majors match. -// This package installs undici 6; the undici built into node varies by -// release line and does not necessarily match that -- today node 22 has -// undici 6 built in (matches), while node 24 has 7 and node 26 has 8 (both -// mismatch, so both already take the fallback path below, not just node 26). -// The contract that breaks (the dispatcher handler's `onError`) changed -// across those majors, so a mismatched pairing rejects the installed v6 -// dispatcher with "invalid onError method". When they match we hand the -// dispatcher to `ky`, which forwards it to the built-in fetch (ky -// deliberately keeps `dispatcher` out of its request-option registry so it -// reaches fetch). When they differ we call the installed undici's own fetch, -// which cannot consume node's built-in `Request` class directly, so it is -// rebuilt as the installed undici's own `Request` first (see `createFetch` -// below). This skew only exists because node does not expose its built-in -// undici (`node:undici`); see digitalbazaar/http-client#43. -// The version read is guarded: if a future undici hides `package.json` behind -// an `exports` map, or `process.versions.undici` is absent, default to the -// installed undici's own fetch (the always-safe path) rather than throwing at +// true when the installed and built-in undici majors match, meaning their +// dispatchers are interchangeable. Both reads are guarded: a future undici +// could hide `package.json` behind an `exports` map, and `versions.undici` +// may be absent. Either way fall back to `false` and use the installed +// undici's own fetch -- the always-safe path -- rather than throwing at // module load and breaking `import` for every consumer. const builtinFetchCompatible = (() => { try { @@ -72,8 +68,9 @@ export function convertAgent(options) { delete rest.agent; delete rest.httpsAgent; - // compatible: let `ky` forward the dispatcher to node's built-in `fetch`, - // which consumes its own `Request` natively — no wrapper, native perf + // majors match: hand the dispatcher to `ky`, which forwards it to the + // built-in `fetch` (`ky` deliberately keeps `dispatcher` out of its + // request-option registry so it reaches fetch) -- no wrapper needed if(builtinFetchCompatible) { return {...rest, dispatcher}; }