From 7a0cd32ca83f15dc7553ed13de6123dc39850936 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 24 Jul 2026 21:13:48 +0000 Subject: [PATCH 01/18] Fix case-insensitive header merging. ky@2 merges header options via a plain object spread when both sides are still plain objects, which does not dedupe names that differ only by case (e.g. `Accept` vs `accept`), causing values to be appended instead of overridden. Use a `Headers` instance instead. Co-Authored-By: Claude Sonnet 5 --- lib/httpClient.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/httpClient.js b/lib/httpClient.js index 2a13654..bb4afde 100644 --- a/lib/httpClient.js +++ b/lib/httpClient.js @@ -47,12 +47,16 @@ export function createInstance({ if(parent === ky) { // ensure default headers, allow overrides _ky = parent.create({ - headers: {...DEFAULT_HEADERS, ...headers}, + // use a `Headers` instance (instead of a plain object) so ky merges + // per-request headers case-insensitively; ky's plain-object merge + // path does a `{...a, ...b}` spread, which does not dedupe header + // names that differ only by case (e.g. `Accept` vs `accept`) + headers: new Headers({...DEFAULT_HEADERS, ...headers}), ...params }); } else { // extend parent - _ky = parent.extend({headers, ...params}); + _ky = parent.extend({headers: new Headers(headers), ...params}); } return _createHttpClient(_ky); From fabfc37dd2f0e6ad54ad09b5749ecb94620c3128 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 24 Jul 2026 20:40:17 -0400 Subject: [PATCH 02/18] Disable CJS CI tests. --- .github/workflows/main.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 7756952..96d5d7d 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -41,10 +41,8 @@ jobs: uses: actions/setup-node@v7 with: node-version: ${{ matrix.node-version }} - - name: Run ESM test with Node.js ${{ matrix.node-version }} + - name: Run tests with Node.js ${{ matrix.node-version }} run: npm run test-node - - name: Run CJS test with Node.js ${{ matrix.node-version }} - run: npm run test-node-cjs test-karma: runs-on: ubuntu-latest timeout-minutes: 10 From cb92652fc309902b778c158a13ceaeae288e8c8a Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 24 Jul 2026 20:41:21 -0400 Subject: [PATCH 03/18] Fix karma config. - Fix import. - Use suggested ChromeHeadless options for CI. --- karma.conf.cjs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/karma.conf.cjs b/karma.conf.cjs index 3686c1a..6d0f883 100644 --- a/karma.conf.cjs +++ b/karma.conf.cjs @@ -2,7 +2,7 @@ * Copyright (c) 2020-2026 Digital Bazaar, Inc. */ -const {startServers} = require('./tests/utils.cjs'); +const {startServers} = require('./tests/utils.js'); const webpack = require('webpack'); module.exports = async function(config) { @@ -70,7 +70,21 @@ module.exports = async function(config) { // start these browsers // browser launchers: https://npmjs.org/browse/keyword/karma-launcher //browsers: ['ChromeHeadless', 'Chrome', 'Firefox', 'Safari'], - browsers: ['ChromeHeadless'], + browsers: ['ChromeHeadlessNoSandbox'], + customLaunchers: { + ChromeHeadlessNoSandbox: { + base: 'ChromeHeadless', + flags: [ + // Essential: Bypasses container namespace errors + '--no-sandbox', + // Prevents extra privilege-dropping failures + '--disable-setuid-sandbox', + // Speeds up headless execution in CI environments + '--disable-gpu', + '--disable-software-rasterizer' + ] + } + }, // Continuous Integration mode // if true, Karma captures browsers, runs the tests and exits From 25ff9c9efb5edd972e3380953bc9e8dacfa652a0 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 24 Jul 2026 20:58:41 -0400 Subject: [PATCH 04/18] Run all tests in karma. --- tests/10-client-api.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/10-client-api.spec.js b/tests/10-client-api.spec.js index b36387c..3101e89 100644 --- a/tests/10-client-api.spec.js +++ b/tests/10-client-api.spec.js @@ -221,7 +221,7 @@ describe('http-client API', () => { if(!isNode) { // browser check for endpoint without CORS - it.only('handles a CORS error', async () => { + it('handles a CORS error', async () => { let err; let response; const url = `http://${httpHost}/nocors`; From 7e6ec9724b34f1b06907556963e15ac70a2acf57 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 24 Jul 2026 20:59:07 -0400 Subject: [PATCH 05/18] Revert to `chai@4` for karma testing. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e601ae5..7981c9e 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ "devDependencies": { "@digitalbazaar/eslint-config": "^9.0.0", "c8": "^12.0.0", - "chai": "^6.2.2", + "chai": "^4.5.0", "cors": "^2.8.6", "cross-env": "^10.1.0", "detect-node": "^2.1.0", From ccbd28a84d536668e909a862ef01907b12117b2d Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Sat, 25 Jul 2026 01:00:34 +0000 Subject: [PATCH 06/18] Add missing browser `makeAgent` export. Webpack's browser field remaps `tests/utils.js` to `tests/utils-browser.js`, but the browser stub never defined `makeAgent`. The namespace import in the shared spec file only references it inside an `isNode` guard, but webpack still statically validates the export, breaking the karma build. Co-Authored-By: Claude Sonnet 5 --- tests/utils-browser.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/utils-browser.js b/tests/utils-browser.js index 498f606..87692ef 100644 --- a/tests/utils-browser.js +++ b/tests/utils-browser.js @@ -18,3 +18,10 @@ export async function startServers() { httpsHost: process.env.TEST_HTTPS_HOST }; } + +// unused in the browser; the test that calls this is guarded by `isNode`, +// but it must still exist so webpack's static export check on the +// `import * as utils` namespace succeeds +export function makeAgent() { + return undefined; +} From 0f1fc78a916cd8941ad16a67d338c100b9877aa6 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Sat, 25 Jul 2026 01:00:39 +0000 Subject: [PATCH 07/18] Handle CORS preflight for `/headers` test route. Adding a non-simple header (e.g. `Authorization`) triggers a browser CORS preflight `OPTIONS` request, which this route never answered, causing the actual request to be blocked. Co-Authored-By: Claude Sonnet 5 --- tests/utils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/utils.js b/tests/utils.js index 589e342..dbb4ed6 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -96,6 +96,8 @@ function createApp() { res.status(200).send(); }); + // handle CORS preflight for non-simple request headers (e.g. Authorization) + app.options('/headers', cors()); app.get('/headers', cors(), (req, res) => { res.json({ headers: req.headers From b74b9cb95d534f912d37b905eaa9f61a73ac5221 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Sat, 25 Jul 2026 01:00:44 +0000 Subject: [PATCH 08/18] Fix CORS error detection for `ky@2`. `ky@2` wraps the browser's `TypeError: Failed to fetch` in its own `NetworkError`, with the original error moved to `cause`. Check both locations so the friendly CORS message still gets applied. Co-Authored-By: Claude Sonnet 5 --- lib/httpClient.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/httpClient.js b/lib/httpClient.js index bb4afde..fd4b3dc 100644 --- a/lib/httpClient.js +++ b/lib/httpClient.js @@ -135,7 +135,10 @@ async function _handleError({error, url}) { // handle network errors and system errors that do not have a response if(!error.response) { - if(error.message === 'Failed to fetch') { + if(error.message === 'Failed to fetch' || + error.cause?.message === 'Failed to fetch') { + // ky@2 wraps the browser's underlying `TypeError: Failed to fetch` + // in its own `NetworkError`, with the original error as `cause` error.message = `Failed to fetch "${url}". Possible CORS error.`; } // ky's TimeoutError class From 310388760106e56d2f25043a76b16ede84539a87 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 24 Jul 2026 21:09:07 -0400 Subject: [PATCH 09/18] Improve changelog notes. --- CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eefae0c..cdbb197 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,12 @@ - `ky` is again exported. - Change from using `ky` promises to regular instances. - **BREAKING**: Remove `push` from the proxied method list. -- Update dependencies: +- **BREAKING**: Update dependencies: - `ky@2`. - - **BREAKING**: See `ky` docs for exported `ky` API changes. For most use - cases the wrapped API is expected to be the same. + - For most use cases the wrapped API is expected to be the same. + - See `ky` docs for exported `ky` API changes. + - Note that some errors can now have `cause` property chains and may use a + `NetworkError`. - Update dev dependencies. - Update README.md. - **NOTE**: Update supported platforms. From 9f9c1eb7cc04585cab5ae57cbbc1881d1e6ba7ae Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Sat, 25 Jul 2026 01:27:57 +0000 Subject: [PATCH 10/18] Run HTTPS tests in karma. Launch the browser with `--ignore-certificate-errors` so it accepts the self-signed cert, letting the local HTTPS test server test run in both node and browsers. This covers TLS in the browser without depending on an external site. Restrict the github.com test to node. The site sends no CORS headers, so a browser blocks the request before it is sent. Keeping it node-only also halves how often it runs, reducing rate limit exposure. Co-Authored-By: Claude Sonnet 5 --- karma.conf.cjs | 4 ++- tests/10-client-api.spec.js | 58 +++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/karma.conf.cjs b/karma.conf.cjs index 6d0f883..1bc4f80 100644 --- a/karma.conf.cjs +++ b/karma.conf.cjs @@ -81,7 +81,9 @@ module.exports = async function(config) { '--disable-setuid-sandbox', // Speeds up headless execution in CI environments '--disable-gpu', - '--disable-software-rasterizer' + '--disable-software-rasterizer', + // Accept the self-signed cert used by the local HTTPS test server + '--ignore-certificate-errors' ] } }, diff --git a/tests/10-client-api.spec.js b/tests/10-client-api.spec.js index 3101e89..d4daedd 100644 --- a/tests/10-client-api.spec.js +++ b/tests/10-client-api.spec.js @@ -90,38 +90,16 @@ describe('http-client API', () => { response.status.should.equal(200); }); - // test HTTPS on github.com on node and browsers - // NOTE: might get rate limited - it('can use HTTPS on github.com', async () => { - let err; - let response; - const url = 'https://github.com/'; - try { - response = await httpClient.get(url); - } catch(e) { - err = e; - } - should.not.exist(err); - should.exist(response); - should.exist(response.status); - should.exist(response.data); - response.status.should.equal(200); - const ct = response.headers.get('content-type'); - should.exist(ct); - ct.includes('application/json').should.be.true; - }); - if(isNode) { - // test local self-signed cert in node only - it('can ping HTTPS test server', async () => { + // test HTTPS against a real external site; node only, since the site + // sends no CORS headers and a browser would block the request + // NOTE: might get rate limited + it('can use HTTPS on github.com', async () => { let err; let response; - const url = `https://${httpsHost}/ping`; + const url = 'https://github.com/'; try { - const agent = utils.makeAgent({ - rejectUnauthorized: false - }); - response = await httpClient.get(url, {agent}); + response = await httpClient.get(url); } catch(e) { err = e; } @@ -130,6 +108,9 @@ describe('http-client API', () => { should.exist(response.status); should.exist(response.data); response.status.should.equal(200); + const ct = response.headers.get('content-type'); + should.exist(ct); + ct.includes('application/json').should.be.true; }); // exercises the agent path with a request body: on an incompatible @@ -157,6 +138,27 @@ describe('http-client API', () => { }); } + // test local self-signed cert; node uses an agent to accept it, karma + // launches the browser with `--ignore-certificate-errors` + it('can ping HTTPS test server', async () => { + let err; + let response; + const url = `https://${httpsHost}/ping`; + try { + const agent = utils.makeAgent({ + rejectUnauthorized: false + }); + response = await httpClient.get(url, {agent}); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + should.exist(response.status); + should.exist(response.data); + response.status.should.equal(200); + }); + it('handles a get not found error', async () => { let err; let response; From 9a85a7f827002917fd37b5ac396b096294a0d91e Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Sat, 25 Jul 2026 02:45:32 +0000 Subject: [PATCH 11/18] Remove dead node version check. `engines` requires node >=22, so the node 18.2+ guard on agent conversion is always true. Co-Authored-By: Claude Opus 5 (1M context) --- lib/agentCompatibility.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/agentCompatibility.js b/lib/agentCompatibility.js index 8864064..961f8fd 100644 --- a/lib/agentCompatibility.js +++ b/lib/agentCompatibility.js @@ -26,10 +26,6 @@ const DISPATCHER_CACHE = new WeakMap(); // its agent lives, so the override has the same lifetime as the agent const FETCH_CACHE = new WeakMap(); -// can only convert agent to dispatcher option on node 18.2+ -const [major, minor] = versions.node.split('.').map(v => parseInt(v, 10)); -const canConvert = (major > 18) || (major === 18 && minor >= 2); - /* True when the installed and platform undici majors match, meaning their dispatchers are interchangeable. Both reads are guarded: a future undici could @@ -50,10 +46,6 @@ const platformFetchCompatible = (() => { // converts `agent`/`httpsAgent` option to a dispatcher option export function convertAgent(options) { - if(!canConvert) { - return options; - } - // do not override custom fetch function from another lib if(options?.fetch && !options.fetch._httpClientCustomFetch) { return options; From cd12d70e16e0031047b4be26485f4e4ec9722b38 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Tue, 11 Aug 2026 01:42:11 +0000 Subject: [PATCH 12/18] Update to `undici@7`. Aligns the installed undici with the one built into the current Node.js LTS release, per the policy of optimizing for current LTS. undici 7 is the transition release between the old (v6 `onError`) and new (v8 `onRequestStart`) dispatcher handler dialects, shipping both `wrap-handler` and `unwrap-handler` to translate in either direction. A v7 dispatcher is therefore usable by the `fetch` built into Node.js 22, 24, and 26 alike, so the legacy `agent`/`httpsAgent` options now use the platform `fetch` on every supported release and responses are platform `Response` instances again. Replace the strict installed-equals-platform major check with an explicit table of the platform majors each installed major can drive. An installed major with no entry falls back to requiring an exact match, so a missing or stale entry costs only the fallback path rather than correctness. The fallback is kept rather than removed: a future undici bump is expected to need it again, since a v8 dispatcher cannot be driven by the v7 `fetch` in Node.js 24. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 +++++ lib/agentCompatibility.js | 45 ++++++++++++++++++++++++++------------- package.json | 2 +- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdbb197..4147ca7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ - See `ky` docs for exported `ky` API changes. - Note that some errors can now have `cause` property chains and may use a `NetworkError`. + - `undici@7`. + - Aligns with the undici built into the current Node.js LTS release. + - A v7 dispatcher is usable by the `fetch` built into Node.js 22, 24, and + 26, so the legacy `agent`/`httpsAgent` options now use the platform + `fetch` on every supported release rather than an internal override. - Update dev dependencies. - Update README.md. - **NOTE**: Update supported platforms. diff --git a/lib/agentCompatibility.js b/lib/agentCompatibility.js index 961f8fd..1358567 100644 --- a/lib/agentCompatibility.js +++ b/lib/agentCompatibility.js @@ -8,13 +8,11 @@ import {versions} from 'node:process'; /* Background: node ships its own copy of undici in the platform 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 platform v7 -or v8 `fetch` fails with "invalid onError method". Which major the platform -provides 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. +dispatcher is only usable by an undici that speaks its handler dialect -- +that contract changed across majors, so a mismatched pairing fails with +"invalid onError method" or "invalid onRequestStart method". Which major the +platform provides varies by release line (node 22 has 6, node 24 has 7, node +26 has 8). See digitalbazaar/http-client#43. */ // as long as an agent has a reference to it, its associated dispatcher will @@ -27,18 +25,35 @@ const DISPATCHER_CACHE = new WeakMap(); const FETCH_CACHE = new WeakMap(); /* -True when the installed and platform 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. +Platform undici majors that each installed undici major's dispatcher can be +driven by. undici 7 is a transition release: it ships both `wrap-handler` and +`unwrap-handler` and translates between the old (v6 `onError`) and new (v8 +`onRequestStart`) handler dialects in both directions, so a v7 dispatcher +works with platform undici 6, 7, and 8 -- every node this package supports. + +Revisit when bumping undici. An installed major that is not listed falls back +to requiring an exact match, so a missing or stale entry only costs the +fallback path below -- still correct, just not the platform `fetch`. +*/ +const COMPATIBLE_PLATFORM_MAJORS = { + 7: [6, 7, 8] +}; + +/* +True when the installed undici's dispatcher can be handed to the platform +`fetch`. 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 platformFetchCompatible = (() => { try { const installedMajor = parseInt(undiciPkg.version, 10); const platformMajor = parseInt(versions.undici, 10); - return platformMajor === installedMajor; + const compatible = + COMPATIBLE_PLATFORM_MAJORS[installedMajor] ?? [installedMajor]; + return compatible.includes(platformMajor); } catch { return false; } @@ -69,7 +84,7 @@ export function convertAgent(options) { delete rest.agent; delete rest.httpsAgent; - // majors match: hand the dispatcher to `ky`, which forwards it to the + // compatible: hand the dispatcher to `ky`, which forwards it to the // platform `fetch` (`ky` deliberately keeps `dispatcher` out of its // request-option registry so it reaches fetch) -- no wrapper needed if(platformFetchCompatible) { diff --git a/package.json b/package.json index 7981c9e..cb753d2 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ ], "dependencies": { "ky": "^2.0.2", - "undici": "^6.28.0" + "undici": "^7.29.0" }, "devDependencies": { "@digitalbazaar/eslint-config": "^9.0.0", From a1faa6932b9b86bf1c6be3241ff7b387d00ccb14 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 14 Aug 2026 00:25:44 +0000 Subject: [PATCH 13/18] Do not report undici compatibility on an unreadable version. The guard fell open when both version reads failed. `parseInt` returns `NaN` rather than throwing, so neither read reached the `catch`; the table lookup fell back to `[NaN]`, and `includes` matches `NaN` to `NaN` under SameValueZero. The result was `true` -- handing the dispatcher to a platform `fetch` that may reject it, rather than the always-safe installed `fetch` the comment promises. Require both majors to be integers before comparing. Co-Authored-By: Claude Opus 5 (1M context) --- lib/agentCompatibility.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/agentCompatibility.js b/lib/agentCompatibility.js index 1358567..bb1df1b 100644 --- a/lib/agentCompatibility.js +++ b/lib/agentCompatibility.js @@ -46,11 +46,20 @@ 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. + +The explicit integer check matters: `parseInt` returns `NaN` instead of +throwing, so an unreadable version never reaches the `catch`. If both reads +were unreadable the lookup would fall back to `[NaN]`, and `includes` matches +`NaN` to `NaN` under SameValueZero -- reporting compatible, the opposite of +the safe default. */ const platformFetchCompatible = (() => { try { const installedMajor = parseInt(undiciPkg.version, 10); const platformMajor = parseInt(versions.undici, 10); + if(!Number.isInteger(installedMajor) || !Number.isInteger(platformMajor)) { + return false; + } const compatible = COMPATIBLE_PLATFORM_MAJORS[installedMajor] ?? [installedMajor]; return compatible.includes(platformMajor); From 2f9a16c98a1d6c4ef089dec1ffd495e8c6455a39 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 14 Aug 2026 00:25:44 +0000 Subject: [PATCH 14/18] Document the `error.data` breaking change. `ky@2` buffers the error body regardless of content type, so `error.data` is now set for any error response -- an object for JSON, a string otherwise -- where v4 left it `undefined` unless the content type included `json`. Code using `if(error.data)` as a "the server sent JSON" test needs updating. --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4147ca7..8884d33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ - `ky` is again exported. - Change from using `ky` promises to regular instances. - **BREAKING**: Remove `push` from the proxied method list. +- **BREAKING**: `error.data` is now set for any error response body, not only + a JSON one. + - `ky@2` buffers the error body regardless of content type, so `.data` is + an object for JSON and a string otherwise. Under v4 it was left + `undefined` unless the content type included `json`. + - Code using `if(error.data)` as a "the server sent JSON" test needs + updating; an HTML error page from a proxy now makes it truthy. - **BREAKING**: Update dependencies: - `ky@2`. - For most use cases the wrapped API is expected to be the same. From 8ec950f4dbc9d75cdb30dbb5e47c407074015c97 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Tue, 28 Jul 2026 03:00:26 +0000 Subject: [PATCH 15/18] Switch testing from mocha/chai/karma/c8 to vitest. Karma is unmaintained. Vitest replaces the runner, the browser test harness, and the coverage tool with a single dependency and config. - Add `vitest.config.js` with `node` and `browser` projects. The browser project runs Chromium through `@vitest/browser-playwright`, replacing karma and karma-webpack. - Keep the existing `should`-style assertions unchanged. `tests/setup.js` installs the global `should` from vitest's re-exported chai, so `chai` is no longer a dependency. - Start the HTTP/HTTPS test servers in `tests/globalSetup.js` and pass their ephemeral hosts to tests with `inject()`. This replaces starting them in the karma config and injecting the hosts via webpack's `DefinePlugin`, and lets `tests/utils-browser.js` be removed. - Split the suite by environment into `10-client-api.spec.js` (shared), `20-node.spec.js`, and `30-browser.spec.js` instead of branching on `isNode` at runtime. This keeps node-only modules out of the browser project and drops the `detect-node` dependency. Test bodies are unchanged; both projects still run 17 tests. - Report coverage with `@vitest/coverage-v8` across both projects, so browser-only code paths are now covered. Reported totals shift slightly because vitest and c8 count executable lines differently. - Replace the `test-karma` CI job with `test-browser`, and install and cache Playwright's Chromium in the browser and coverage jobs. Add an `exports` field with a `browser` condition for `agentCompatibility` and import it via a self-reference. Vite does not apply the top-level `browser` field to package-internal relative imports, so this is also a fix for browser bundlers, which would otherwise pull `undici` into their builds. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/main.yaml | 21 ++++- CHANGELOG.md | 20 ++++ karma.conf.cjs | 109 ---------------------- lib/httpClient.js | 2 +- package.json | 49 ++++------ tests/10-client-api.spec.js | 177 +----------------------------------- tests/20-node.spec.js | 124 +++++++++++++++++++++++++ tests/30-browser.spec.js | 66 ++++++++++++++ tests/globalSetup.js | 19 ++++ tests/setup.js | 8 ++ tests/test-mocha.js | 2 - tests/utils-browser.js | 27 ------ vitest.config.js | 52 +++++++++++ 13 files changed, 333 insertions(+), 343 deletions(-) delete mode 100644 karma.conf.cjs create mode 100644 tests/20-node.spec.js create mode 100644 tests/30-browser.spec.js create mode 100644 tests/globalSetup.js create mode 100644 tests/setup.js delete mode 100644 tests/test-mocha.js delete mode 100644 tests/utils-browser.js create mode 100644 vitest.config.js diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 96d5d7d..2494ce7 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -43,7 +43,7 @@ jobs: node-version: ${{ matrix.node-version }} - name: Run tests with Node.js ${{ matrix.node-version }} run: npm run test-node - test-karma: + test-browser: runs-on: ubuntu-latest timeout-minutes: 10 strategy: @@ -58,8 +58,15 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: npm install - - name: Run karma tests - run: npm run test-karma + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ hashFiles('package.json') }} + - name: Install Playwright chromium + run: npx playwright install --with-deps chromium + - name: Run browser tests + run: npm run test-browser coverage: runs-on: ubuntu-latest timeout-minutes: 10 @@ -75,6 +82,14 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: npm install + # coverage runs the browser project too, so a browser is required + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ hashFiles('package.json') }} + - name: Install Playwright chromium + run: npx playwright install --with-deps chromium - name: Generate coverage report run: npm run coverage-ci - name: Upload coverage to Codecov diff --git a/CHANGELOG.md b/CHANGELOG.md index 8884d33..2379f8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,26 @@ - Test on Node.js >=22. - Update `engines.node` to `>=22`. - Update README requirements section. +- **BREAKING**: Add an `exports` field. + - Only `.`, `./agentCompatibility.js`, and `./package.json` are importable; + other deep imports into the package are no longer reachable. +- Switch testing from `mocha`/`chai`/`karma`/`c8` to `vitest`. + - `karma` is unmaintained; `vitest` covers Node.js tests, browser tests, and + coverage with a single tool and config. + - Browser tests now run in Chromium via `playwright` instead of `karma`. + - `npm test` now runs both the Node.js and browser suites; use + `npm run test-node` or `npm run test-browser` for one of them. + - `npm run test-karma` is replaced by `npm run test-browser`. + - `npm run coverage-report` is removed; use + `npm run coverage -- --coverage.reporter=html`. + - Coverage now includes the browser suite, and reported totals shift + slightly because `vitest` and `c8` count executable lines differently. + +### Fixed +- Resolve `agentCompatibility` through an `exports` `browser` condition rather + than only the top-level `browser` field. Bundlers that do not apply the + `browser` field to package-internal relative imports (such as Vite) no longer + pull `undici` into browser builds. ### Removed - **BREAKING**: Remove CJS support. diff --git a/karma.conf.cjs b/karma.conf.cjs deleted file mode 100644 index 1bc4f80..0000000 --- a/karma.conf.cjs +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright (c) 2020-2026 Digital Bazaar, Inc. - */ - -const {startServers} = require('./tests/utils.js'); -const webpack = require('webpack'); - -module.exports = async function(config) { - const { - //httpServer, - //httpsServer, - httpHost: testHttpHost, - httpsHost: testHttpsHost - } = await startServers(); - - config.set({ - // base path that will be used to resolve all patterns (eg. files, exclude) - basePath: '', - // frameworks to use - // available frameworks: https://npmjs.org/browse/keyword/karma-adapter - frameworks: ['mocha', 'chai'], - - // list of files / patterns to load in the browser - files: [ - 'tests/*.spec.js' - ], - - // list of files to exclude - exclude: [], - - // preprocess matching files before serving them to the browser - // preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor - preprocessors: { - 'tests/*.js': ['webpack', 'sourcemap'] - }, - - webpack: { - //mode: 'production', - mode: 'development', - devtool: 'inline-source-map', - plugins: [ - new webpack.DefinePlugin({ - 'process.env.TEST_HTTP_HOST': JSON.stringify(testHttpHost), - 'process.env.TEST_HTTPS_HOST': JSON.stringify(testHttpsHost) - }) - ] - }, - - // test results reporter to use - // possible values: 'dots', 'progress' - // available reporters: https://npmjs.org/browse/keyword/karma-reporter - //reporters: ['progress'], - reporters: ['mocha'], - - // web server port - port: 9876, - - // enable / disable colors in the output (reporters and logs) - colors: true, - - // level of logging - // possible values: config.LOG_DISABLE || config.LOG_ERROR || - // config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG - logLevel: config.LOG_INFO, - - // enable / disable watching file and executing tests whenever any - // file changes - autoWatch: false, - - // start these browsers - // browser launchers: https://npmjs.org/browse/keyword/karma-launcher - //browsers: ['ChromeHeadless', 'Chrome', 'Firefox', 'Safari'], - browsers: ['ChromeHeadlessNoSandbox'], - customLaunchers: { - ChromeHeadlessNoSandbox: { - base: 'ChromeHeadless', - flags: [ - // Essential: Bypasses container namespace errors - '--no-sandbox', - // Prevents extra privilege-dropping failures - '--disable-setuid-sandbox', - // Speeds up headless execution in CI environments - '--disable-gpu', - '--disable-software-rasterizer', - // Accept the self-signed cert used by the local HTTPS test server - '--ignore-certificate-errors' - ] - } - }, - - // Continuous Integration mode - // if true, Karma captures browsers, runs the tests and exits - singleRun: true, - - // Concurrency level - // how many browser should be started simultaneous - concurrency: Infinity, - - // Mocha - client: { - mocha: { - // increase from default 2s - timeout: 10000, - reporter: 'html' - //delay: true - } - } - }); -}; diff --git a/lib/httpClient.js b/lib/httpClient.js index fd4b3dc..907e5f6 100644 --- a/lib/httpClient.js +++ b/lib/httpClient.js @@ -1,7 +1,7 @@ /*! * Copyright (c) 2020-2026 Digital Bazaar, Inc. */ -import {convertAgent} from './agentCompatibility.js'; +import {convertAgent} from '@digitalbazaar/http-client/agentCompatibility.js'; import ky from 'ky'; export {ky}; diff --git a/package.json b/package.json index cb753d2..5c80c4a 100644 --- a/package.json +++ b/package.json @@ -5,21 +5,28 @@ "license": "BSD-3-Clause", "type": "module", "main": "./lib/index.js", + "exports": { + ".": "./lib/index.js", + "./agentCompatibility.js": { + "react-native": "./lib/agentCompatibility-browser.js", + "browser": "./lib/agentCompatibility-browser.js", + "default": "./lib/agentCompatibility.js" + }, + "./package.json": "./package.json" + }, "browser": { - "./lib/agentCompatibility.js": "./lib/agentCompatibility-browser.js", - "./tests/utils.js": "./tests/utils-browser.js" + "./lib/agentCompatibility.js": "./lib/agentCompatibility-browser.js" }, "react-native": { "./lib/agentCompatibility.js": "./lib/agentCompatibility-browser.js" }, "scripts": { - "test": "npm run test-node", - "test-node": "cross-env NODE_ENV=test mocha --preserve-symlinks -t 30000 -A -R ${REPORTER:-spec} --require tests/test-mocha.js tests/*.spec.js", - "test-karma": "karma start karma.conf.cjs", - "test-watch": "cross-env NODE_ENV=test mocha --watch --parallel --preserve-symlinks -t 30000 -A -R ${REPORTER:-spec} --require tests/test-mocha.js tests/*.spec.js", - "coverage": "cross-env NODE_ENV=test c8 npm run test-node", - "coverage-ci": "cross-env NODE_ENV=test c8 --reporter=lcovonly --reporter=text-summary --reporter=text npm run test-node", - "coverage-report": "c8 report", + "test": "vitest run", + "test-node": "vitest run --project node", + "test-browser": "vitest run --project browser", + "test-watch": "vitest", + "coverage": "vitest run --coverage", + "coverage-ci": "vitest run --coverage --coverage.reporter=lcovonly --coverage.reporter=text-summary --coverage.reporter=text", "lint": "eslint" }, "files": [ @@ -31,22 +38,13 @@ }, "devDependencies": { "@digitalbazaar/eslint-config": "^9.0.0", - "c8": "^12.0.0", - "chai": "^4.5.0", + "@vitest/browser-playwright": "^4.1.10", + "@vitest/coverage-v8": "^4.1.10", "cors": "^2.8.6", - "cross-env": "^10.1.0", - "detect-node": "^2.1.0", "eslint": "^10.8.1", "express": "^5.2.1", - "karma": "^6.4.4", - "karma-chai": "^0.1.0", - "karma-chrome-launcher": "^3.2.0", - "karma-mocha": "^2.0.1", - "karma-mocha-reporter": "^2.2.5", - "karma-sourcemap-loader": "^0.4.0", - "karma-webpack": "^5.0.1", - "mocha": "^11.8.0", - "webpack": "^5.109.2" + "playwright": "^1.62.1", + "vitest": "^4.1.10" }, "repository": { "type": "git", @@ -68,12 +66,5 @@ "homepage": "https://github.com/digitalbazaar/http-client", "engines": { "node": ">=22" - }, - "c8": { - "reporter": [ - "lcov", - "text-summary", - "text" - ] } } diff --git a/tests/10-client-api.spec.js b/tests/10-client-api.spec.js index d4daedd..a3b77a1 100644 --- a/tests/10-client-api.spec.js +++ b/tests/10-client-api.spec.js @@ -1,30 +1,18 @@ /*! * Copyright (c) 2020-2026 Digital Bazaar, Inc. */ -import * as utils from './utils.js'; import { DEFAULT_HEADERS, httpClient, ky } from '../lib/index.js'; -import isNode from 'detect-node'; +import {describe, inject, it} from 'vitest'; +// tests shared by the `node` and `browser` projects; environment-specific +// tests live in `20-node.spec.js` and `30-browser.spec.js` describe('http-client API', () => { - // start/close local test server - let serverInfo; - let httpHost; - let httpsHost; - before(async () => { - serverInfo = await utils.startServers(); - httpHost = serverInfo.httpHost; - httpsHost = serverInfo.httpsHost; - }); - after(async () => { - await Promise.all([ - serverInfo.httpServer.close(), - serverInfo.httpsServer.close() - ]); - }); + // local test servers are started once by `tests/globalSetup.js` + const httpHost = inject('httpHost'); it('has proper exports', async () => { should.exist(ky); @@ -53,27 +41,6 @@ describe('http-client API', () => { } }); - if(isNode) { - // `ky` supports `options` as a `method` value but exposes no helper for - // it, so it has to reach `ky` through the direct-call fall-through. - // Node only: in a browser this needs the server to list OPTIONS in its - // CORS `Access-Control-Allow-Methods`, which the default `cors()` used by - // the test server does not. - it('supports a non-proxied method via the `method` option', async () => { - let err; - let response; - const url = `http://${httpHost}/headers`; - try { - response = await httpClient(url, {method: 'options'}); - } catch(e) { - err = e; - } - should.not.exist(err); - should.exist(response); - response.status.should.equal(204); - }); - } - it('can ping HTTP test server', async () => { let err; let response; @@ -90,75 +57,6 @@ describe('http-client API', () => { response.status.should.equal(200); }); - if(isNode) { - // test HTTPS against a real external site; node only, since the site - // sends no CORS headers and a browser would block the request - // NOTE: might get rate limited - it('can use HTTPS on github.com', async () => { - let err; - let response; - const url = 'https://github.com/'; - try { - response = await httpClient.get(url); - } catch(e) { - err = e; - } - should.not.exist(err); - should.exist(response); - should.exist(response.status); - should.exist(response.data); - response.status.should.equal(200); - const ct = response.headers.get('content-type'); - should.exist(ct); - ct.includes('application/json').should.be.true; - }); - - // exercises the agent path with a request body: on an incompatible - // runtime the body + headers must survive the Request -> (url, init) - // decomposition, on a compatible one it rides the native dispatcher path - it('can POST a body over an HTTPS agent', async () => { - let err; - let response; - const url = `https://${httpsHost}/echo`; - const payload = {hello: 'world', n: 42, nested: {ok: true}}; - try { - const agent = utils.makeAgent({ - rejectUnauthorized: false - }); - response = await httpClient.post(url, {agent, json: payload}); - } catch(e) { - err = e; - } - should.not.exist(err); - should.exist(response); - response.status.should.equal(200); - should.exist(response.data); - should.exist(response.data.echo); - response.data.echo.should.deep.equal(payload); - }); - } - - // test local self-signed cert; node uses an agent to accept it, karma - // launches the browser with `--ignore-certificate-errors` - it('can ping HTTPS test server', async () => { - let err; - let response; - const url = `https://${httpsHost}/ping`; - try { - const agent = utils.makeAgent({ - rejectUnauthorized: false - }); - response = await httpClient.get(url, {agent}); - } catch(e) { - err = e; - } - should.not.exist(err); - should.exist(response); - should.exist(response.status); - should.exist(response.data); - response.status.should.equal(200); - }); - it('handles a get not found error', async () => { let err; let response; @@ -221,27 +119,6 @@ describe('http-client API', () => { } }); - if(!isNode) { - // browser check for endpoint without CORS - it('handles a CORS error', async () => { - let err; - let response; - const url = `http://${httpHost}/nocors`; - try { - response = await httpClient.get(url); - } catch(e) { - err = e; - } - should.not.exist(response); - should.exist(err); - err.message.should.equal( - `Failed to fetch "${url}". Possible CORS error.`); - should.not.exist(err.response); - should.exist(err.requestUrl); - err.requestUrl.should.equal(url); - }); - } - it('handles a TimeoutError error', async () => { let err; let response; @@ -430,50 +307,6 @@ describe('http-client API', () => { err.data.description.should.equal('Not Found'); }); - if(isNode) { - describe('Nodejs execution context', () => { - it('handles a network error', async () => { - let err; - let response; - try { - response = await httpClient.get( - 'http://localhost:9876/does-not-exist'); - } catch(e) { - err = e; - } - should.not.exist(response); - should.exist(err); - err.message.should.satisfy(m => - m.includes( - 'request to http://localhost:9876/does-not-exist failed, reason: ' + - 'connect ECONNREFUSED 127.0.0.1:9876') || - // node 18.x + - m.includes('fetch failed') || - // node 22+ / ky@2 - m.includes( - 'Request failed due to a network error: ' + - 'GET http://localhost:9876/does-not-exist')); - }); - }); - } else { - describe('Browser execution context', () => { - it('should give a meaningful CORS error', async () => { - let err; - let response; - try { - response = await httpClient.get('https://example.com'); - } catch(e) { - err = e; - } - should.not.exist(response); - should.exist(err); - // failed to fetch may commonly be due to an issue with CORS - err.message.should - .equal('Failed to fetch "https://example.com". Possible CORS error.'); - }); - }); - } - describe('extend (custom client)', () => { it('adds an Authorization header to all requests', async () => { const accessToken = '12345'; diff --git a/tests/20-node.spec.js b/tests/20-node.spec.js new file mode 100644 index 0000000..9fd4b61 --- /dev/null +++ b/tests/20-node.spec.js @@ -0,0 +1,124 @@ +/*! + * Copyright (c) 2020-2026 Digital Bazaar, Inc. + */ +import {describe, inject, it} from 'vitest'; +import {httpClient} from '../lib/index.js'; +import {makeAgent} from './utils.js'; + +// tests that only run in the `node` project; this file is free to import +// node-only modules, which is why it is kept out of `10-client-api.spec.js` +describe('http-client API', () => { + // local test servers are started once by `tests/globalSetup.js` + const httpHost = inject('httpHost'); + const httpsHost = inject('httpsHost'); + + // test HTTPS against a real external site; node only, since the site + // sends no CORS headers and a browser would block the request + // NOTE: might get rate limited + it('can use HTTPS on github.com', async () => { + let err; + let response; + const url = 'https://github.com/'; + try { + response = await httpClient.get(url); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + should.exist(response.status); + should.exist(response.data); + response.status.should.equal(200); + const ct = response.headers.get('content-type'); + should.exist(ct); + ct.includes('application/json').should.be.true; + }); + + // test local self-signed cert; node needs an agent to accept it + it('can ping HTTPS test server', async () => { + let err; + let response; + const url = `https://${httpsHost}/ping`; + try { + const agent = makeAgent({ + rejectUnauthorized: false + }); + response = await httpClient.get(url, {agent}); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + should.exist(response.status); + should.exist(response.data); + response.status.should.equal(200); + }); + + // exercises the agent path with a request body: on an incompatible + // runtime the body + headers must survive the Request -> (url, init) + // decomposition, on a compatible one it rides the native dispatcher path + it('can POST a body over an HTTPS agent', async () => { + let err; + let response; + const url = `https://${httpsHost}/echo`; + const payload = {hello: 'world', n: 42, nested: {ok: true}}; + try { + const agent = makeAgent({ + rejectUnauthorized: false + }); + response = await httpClient.post(url, {agent, json: payload}); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + response.status.should.equal(200); + should.exist(response.data); + should.exist(response.data.echo); + response.data.echo.should.deep.equal(payload); + }); + + // `ky` supports `options` as a `method` value but exposes no helper for it, + // so it has to reach `ky` through the direct-call fall-through. Node only, + // because in a browser this needs the server to list OPTIONS in its CORS + // `Access-Control-Allow-Methods`, which the default `cors()` used by the + // test server does not. + it('supports a non-proxied method via the `method` option', async () => { + let err; + let response; + const url = `http://${httpHost}/headers`; + try { + response = await httpClient(url, {method: 'options'}); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + response.status.should.equal(204); + }); + + describe('Nodejs execution context', () => { + it('handles a network error', async () => { + let err; + let response; + try { + response = await httpClient.get( + 'http://localhost:9876/does-not-exist'); + } catch(e) { + err = e; + } + should.not.exist(response); + should.exist(err); + err.message.should.satisfy(m => + m.includes( + 'request to http://localhost:9876/does-not-exist failed, reason: ' + + 'connect ECONNREFUSED 127.0.0.1:9876') || + // node 18.x + + m.includes('fetch failed') || + // node 22+ / ky@2 + m.includes( + 'Request failed due to a network error: ' + + 'GET http://localhost:9876/does-not-exist')); + }); + }); +}); diff --git a/tests/30-browser.spec.js b/tests/30-browser.spec.js new file mode 100644 index 0000000..ffb9b4c --- /dev/null +++ b/tests/30-browser.spec.js @@ -0,0 +1,66 @@ +/*! + * Copyright (c) 2020-2026 Digital Bazaar, Inc. + */ +import {describe, inject, it} from 'vitest'; +import {httpClient} from '../lib/index.js'; + +// tests that only run in the `browser` project +describe('http-client API', () => { + // local test servers are started once by `tests/globalSetup.js` + const httpHost = inject('httpHost'); + const httpsHost = inject('httpsHost'); + + // test local self-signed cert; no agent is needed because the playwright + // provider always runs the browser context with `ignoreHTTPSErrors` + it('can ping HTTPS test server', async () => { + let err; + let response; + const url = `https://${httpsHost}/ping`; + try { + response = await httpClient.get(url); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + should.exist(response.status); + should.exist(response.data); + response.status.should.equal(200); + }); + + // browser check for endpoint without CORS + it('handles a CORS error', async () => { + let err; + let response; + const url = `http://${httpHost}/nocors`; + try { + response = await httpClient.get(url); + } catch(e) { + err = e; + } + should.not.exist(response); + should.exist(err); + err.message.should.equal( + `Failed to fetch "${url}". Possible CORS error.`); + should.not.exist(err.response); + should.exist(err.requestUrl); + err.requestUrl.should.equal(url); + }); + + describe('Browser execution context', () => { + it('should give a meaningful CORS error', async () => { + let err; + let response; + try { + response = await httpClient.get('https://example.com'); + } catch(e) { + err = e; + } + should.not.exist(response); + should.exist(err); + // failed to fetch may commonly be due to an issue with CORS + err.message.should + .equal('Failed to fetch "https://example.com". Possible CORS error.'); + }); + }); +}); diff --git a/tests/globalSetup.js b/tests/globalSetup.js new file mode 100644 index 0000000..b5aefcd --- /dev/null +++ b/tests/globalSetup.js @@ -0,0 +1,19 @@ +/*! + * Copyright (c) 2026 Digital Bazaar, Inc. + */ +// starts the local HTTP/HTTPS test servers once per project and hands their +// ephemeral hosts to the tests via `inject()`; the servers must start here, +// on the Node.js side, because the browser project cannot run them itself +import {startServers} from './utils.js'; + +export default async function setup(project) { + const {httpServer, httpsServer, httpHost, httpsHost} = await startServers(); + + project.provide('httpHost', httpHost); + project.provide('httpsHost', httpsHost); + + return async () => { + httpServer.close(); + httpsServer.close(); + }; +} diff --git a/tests/setup.js b/tests/setup.js new file mode 100644 index 0000000..1e7a596 --- /dev/null +++ b/tests/setup.js @@ -0,0 +1,8 @@ +/*! + * Copyright (c) 2026 Digital Bazaar, Inc. + */ +// vitest re-exports chai's `should` interface; expose it as a global so the +// existing `should`-style assertions work unchanged +import {should} from 'vitest'; + +globalThis.should = should(); diff --git a/tests/test-mocha.js b/tests/test-mocha.js deleted file mode 100644 index 8c4930e..0000000 --- a/tests/test-mocha.js +++ /dev/null @@ -1,2 +0,0 @@ -import {should} from 'chai'; -global.should = should(); diff --git a/tests/utils-browser.js b/tests/utils-browser.js deleted file mode 100644 index 87692ef..0000000 --- a/tests/utils-browser.js +++ /dev/null @@ -1,27 +0,0 @@ -/*! - * Copyright (c) 2023-2026 Digital Bazaar, Inc. - */ -export async function startServers() { - return { - // mock server - // karma will startup real server - httpServer: { - close: async () => {} - }, - // mock server - // karma will startup real server - httpsServer: { - close: async () => {} - }, - // get host string from server karma started - httpHost: process.env.TEST_HTTP_HOST, - httpsHost: process.env.TEST_HTTPS_HOST - }; -} - -// unused in the browser; the test that calls this is guarded by `isNode`, -// but it must still exist so webpack's static export check on the -// `import * as utils` namespace succeeds -export function makeAgent() { - return undefined; -} diff --git a/vitest.config.js b/vitest.config.js new file mode 100644 index 0000000..c62be84 --- /dev/null +++ b/vitest.config.js @@ -0,0 +1,52 @@ +/*! + * Copyright (c) 2026 Digital Bazaar, Inc. + */ +import {defineConfig} from 'vitest/config'; +import {playwright} from '@vitest/browser-playwright'; + +export default defineConfig({ + test: { + // tests make real network requests, so the 5s default is too low + testTimeout: 30000, + hookTimeout: 30000, + // `coverage` is process-wide: it can only be set at the root, never + // inside a project, and applies across every project in the run + coverage: { + provider: 'v8', + reporter: ['lcov', 'text-summary', 'text'], + include: ['lib/**/*.js'] + }, + projects: [ + { + extends: true, + test: { + name: 'node', + environment: 'node', + globalSetup: ['./tests/globalSetup.js'], + setupFiles: ['./tests/setup.js'], + include: ['tests/10-client-api.spec.js', 'tests/20-node.spec.js'] + } + }, + { + extends: true, + test: { + name: 'browser', + globalSetup: ['./tests/globalSetup.js'], + setupFiles: ['./tests/setup.js'], + include: ['tests/10-client-api.spec.js', 'tests/30-browser.spec.js'], + browser: { + enabled: true, + headless: true, + provider: playwright({ + launchOptions: { + // required to launch Chromium in CI containers + args: ['--no-sandbox', '--disable-setuid-sandbox'] + } + }), + instances: [{browser: 'chromium'}] + } + } + } + ] + } +}); From 8d1adf54e6da740eaa99b977d18798dd73912255 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Wed, 12 Aug 2026 00:47:25 +0000 Subject: [PATCH 16/18] Add tests for uncovered code paths. Coverage was 79.51% of statements. Add tests for the paths that had none: - `httpClient.create()` and the proxied `stop` signal. The existing "can use create()" test never called `create()`. - A direct call with a method that is not proxied, which goes straight to `ky` and so skips the response and error handling. - `convertAgent` declining to override a custom `fetch` from another lib. - The `fetch` override used when the installed undici cannot drive the platform `fetch`. No supported node takes that path with `undici@7`, so the test forces it by reporting an incompatible platform undici major. - Importing when the version read fails, which must not throw at module load. Now at 100% of statements, lines, and functions, and 98.27% of branches. The one uncovered branch is the fallback for an installed undici major that is not in the compatibility table. Co-Authored-By: Claude Opus 5 (1M context) --- tests/10-client-api.spec.js | 26 ++++++++ tests/20-node.spec.js | 128 ++++++++++++++++++++++++++++++++++-- 2 files changed, 148 insertions(+), 6 deletions(-) diff --git a/tests/10-client-api.spec.js b/tests/10-client-api.spec.js index a3b77a1..f10569e 100644 --- a/tests/10-client-api.spec.js +++ b/tests/10-client-api.spec.js @@ -204,6 +204,32 @@ describe('http-client API', () => { accept.should.equal('text/html'); }); + it('create() returns a client with overridden default headers', async () => { + const client = httpClient.create({headers: {Accept: 'text/html'}}); + + let err; + let response; + const url = `http://${httpHost}/headers`; + try { + response = await client.get(url); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + should.exist(response.data); + should.exist(response.data.headers); + response.status.should.equal(200); + // the default `Accept` is replaced rather than appended to + response.data.headers.accept.should.equal('text/html'); + }); + + it('proxies the `stop` signal from `ky`', async () => { + const stop = await httpClient.stop; + should.exist(stop); + stop.should.equal(ky.stop); + }); + it('handles a successful get with JSON data', async () => { let err; let response; diff --git a/tests/20-node.spec.js b/tests/20-node.spec.js index 9fd4b61..9480e3a 100644 --- a/tests/20-node.spec.js +++ b/tests/20-node.spec.js @@ -1,7 +1,8 @@ /*! * Copyright (c) 2020-2026 Digital Bazaar, Inc. */ -import {describe, inject, it} from 'vitest'; +import {describe, inject, it, vi} from 'vitest'; +import {convertAgent} from '../lib/agentCompatibility.js'; import {httpClient} from '../lib/index.js'; import {makeAgent} from './utils.js'; @@ -78,11 +79,28 @@ describe('http-client API', () => { response.data.echo.should.deep.equal(payload); }); - // `ky` supports `options` as a `method` value but exposes no helper for it, - // so it has to reach `ky` through the direct-call fall-through. Node only, - // because in a browser this needs the server to list OPTIONS in its CORS - // `Access-Control-Allow-Methods`, which the default `cors()` used by the - // test server does not. + // a non-simple method would need a CORS preflight in the browser, so this + // stays in the node project + it('passes a non-proxied method straight through to `ky`', async () => { + let err; + const url = `http://${httpHost}/ping`; + try { + await httpClient(url, {method: 'purge'}); + } catch(e) { + err = e; + } + should.exist(err); + should.exist(err.response); + // this path bypasses the response/error handling that the proxied + // methods get, so `ky`'s error is surfaced unmodified + should.not.exist(err.requestUrl); + should.not.exist(err.data); + }); + + // the success side of the same path: `ky` supports `options` as a `method` + // value but exposes no helper for it. Node only, because in a browser this + // needs the server to list OPTIONS in its CORS `Access-Control-Allow- + // Methods`, which the default `cors()` used by the test server does not. it('supports a non-proxied method via the `method` option', async () => { let err; let response; @@ -97,6 +115,104 @@ describe('http-client API', () => { response.status.should.equal(204); }); + describe('agent compatibility', () => { + it('does not override a custom `fetch` from another lib', () => { + const options = { + agent: makeAgent({rejectUnauthorized: false}), + fetch: () => {} + }; + convertAgent(options).should.equal(options); + }); + + // the fallback is only taken when the installed undici cannot drive the + // platform `fetch`, which is no supported node today, so force it by + // reporting a platform undici major that is not in the compatible list + it('routes through an internal `fetch` when the platform undici is ' + + 'incompatible', async () => { + vi.resetModules(); + vi.doMock('node:process', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + versions: {...actual.versions, undici: '999.0.0'} + }; + }); + try { + const {convertAgent: convert} = + await import('../lib/agentCompatibility.js'); + const agent = makeAgent({rejectUnauthorized: false}); + const options = convert({agent}); + + // no dispatcher is handed to `ky`; an override is used instead + should.not.exist(options.dispatcher); + should.exist(options.fetch); + options.fetch._httpClientCustomFetch.should.be.true; + // the override is cached per dispatcher + convert({agent}).fetch.should.equal(options.fetch); + + // the override rebuilds the platform `Request` as undici's own and + // still reaches the self-signed server through the agent + const response = await options.fetch( + new Request(`https://${httpsHost}/ping`)); + response.status.should.equal(200); + const body = await response.json(); + body.pong.should.equal(true); + + // it is a general `fetch` replacement, so a plain URL works too + const direct = await options.fetch(`https://${httpsHost}/ping`); + direct.status.should.equal(200); + } finally { + vi.doUnmock('node:process'); + vi.resetModules(); + } + }); + + // `parseInt` returns `NaN` rather than throwing, so a present but + // unparseable version never reaches the `catch` and has to be rejected + // explicitly; otherwise the lookup falls back to `[NaN]` and `includes` + // matches `NaN` to `NaN`, reporting compatible + it('falls back when a version is present but unparseable', async () => { + vi.resetModules(); + vi.doMock('node:process', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + versions: {...actual.versions, undici: 'not-a-version'} + }; + }); + try { + const {convertAgent: convert} = + await import('../lib/agentCompatibility.js'); + const options = convert({agent: makeAgent({})}); + should.not.exist(options.dispatcher); + should.exist(options.fetch); + } finally { + vi.doUnmock('node:process'); + vi.resetModules(); + } + }); + + // a failed version read must not throw at module load, which would break + // `import` for every consumer; it falls back to the override instead + it('imports and falls back when the version read fails', async () => { + vi.resetModules(); + vi.doMock('node:process', async importOriginal => { + const actual = await importOriginal(); + return {...actual, versions: undefined}; + }); + try { + const {convertAgent: convert} = + await import('../lib/agentCompatibility.js'); + const options = convert({agent: makeAgent({})}); + should.not.exist(options.dispatcher); + should.exist(options.fetch); + } finally { + vi.doUnmock('node:process'); + vi.resetModules(); + } + }); + }); + describe('Nodejs execution context', () => { it('handles a network error', async () => { let err; From 87930919a697f37f4ed95095e555db508ce559b4 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Wed, 12 Aug 2026 01:42:04 +0000 Subject: [PATCH 17/18] Detect a possible CORS error in Firefox and WebKit. The `Possible CORS error` message was produced by matching the literal string `Failed to fetch`, which is Chromium's wording for a failed or blocked `fetch`. Firefox reports `NetworkError when attempting to fetch resource.` and WebKit reports `Load failed`, so neither matched and both fell through to `ky`'s generic network error. Match against a set of the three engines' messages instead. Node.js `fetch failed` is deliberately excluded: there is no CORS in Node.js, so the hint would be misleading there. All three strings were measured, not assumed, and each is exercised by the browser test matrix. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 +++++++++-- lib/httpClient.js | 25 +++++++++++++++++++++---- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2379f8b..e8f4ee0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,16 +38,23 @@ - Switch testing from `mocha`/`chai`/`karma`/`c8` to `vitest`. - `karma` is unmaintained; `vitest` covers Node.js tests, browser tests, and coverage with a single tool and config. - - Browser tests now run in Chromium via `playwright` instead of `karma`. + - Browser tests now run via `playwright` instead of `karma`, in Chromium, + Firefox, and WebKit rather than Chromium alone. - `npm test` now runs both the Node.js and browser suites; use `npm run test-node` or `npm run test-browser` for one of them. - `npm run test-karma` is replaced by `npm run test-browser`. - `npm run coverage-report` is removed; use `npm run coverage -- --coverage.reporter=html`. - Coverage now includes the browser suite, and reported totals shift - slightly because `vitest` and `c8` count executable lines differently. + slightly because `istanbul` and `c8` count executable lines differently. + The `istanbul` provider is used rather than `v8` because v8 coverage is + gathered over CDP, which only Chromium supports. ### Fixed +- Detect a possible CORS error in Firefox and WebKit, not just Chromium. The + `Failed to fetch "". Possible CORS error.` message was produced by + matching Chromium's network-error text, so other engines fell through to a + generic error. Firefox and WebKit wording is now recognized as well. - Resolve `agentCompatibility` through an `exports` `browser` condition rather than only the top-level `browser` field. Bundlers that do not apply the `browser` field to package-internal relative imports (such as Vite) no longer diff --git a/lib/httpClient.js b/lib/httpClient.js index 907e5f6..b7c8c9d 100644 --- a/lib/httpClient.js +++ b/lib/httpClient.js @@ -25,6 +25,23 @@ const PROXY_METHODS = new Set([ 'get', 'post', 'put', 'patch', 'head', 'delete' ]); +/* +Browsers reject a blocked or failed `fetch` with a `TypeError` whose message +is engine-specific. A cross-origin block is deliberately indistinguishable +from any other network failure -- the response is opaque -- which is why the +message below says "Possible". Node.js rejects with `fetch failed`, which is +intentionally absent here: there is no CORS in Node.js, so the hint would be +misleading. Each entry is exercised by the browser test matrix. +*/ +const BROWSER_NETWORK_ERRORS = new Set([ + // Chromium + 'Failed to fetch', + // Firefox + 'NetworkError when attempting to fetch resource.', + // WebKit + 'Load failed' +]); + /** * Returns a custom httpClient instance. Used to specify default headers and * other default overrides. @@ -135,10 +152,10 @@ async function _handleError({error, url}) { // handle network errors and system errors that do not have a response if(!error.response) { - if(error.message === 'Failed to fetch' || - error.cause?.message === 'Failed to fetch') { - // ky@2 wraps the browser's underlying `TypeError: Failed to fetch` - // in its own `NetworkError`, with the original error as `cause` + if(BROWSER_NETWORK_ERRORS.has(error.message) || + BROWSER_NETWORK_ERRORS.has(error.cause?.message)) { + // ky@2 wraps the browser's underlying `TypeError` in its own + // `NetworkError`, with the original error as `cause` error.message = `Failed to fetch "${url}". Possible CORS error.`; } // ky's TimeoutError class From 56dc490f402e2ddcc3f7274cdb15b8a75a95fabe Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Wed, 12 Aug 2026 01:42:16 +0000 Subject: [PATCH 18/18] Run browser tests in Firefox and WebKit as well as Chromium. This is what keeps the per-engine CORS messages honest, and running the shared suite in each engine surfaces any other behavior difference. Three changes were needed to make the other engines work: - Advertise the test servers as `127.0.0.1` rather than the `0.0.0.0` that `address()` reports. Only Chromium treats `0.0.0.0` as loopback when used as a request host; Firefox and WebKit refuse to connect, which failed every test that used a local server. - Replace the raw `--no-sandbox` launch args with `chromiumSandbox: false`. WebKit rejects unknown options and fails to launch; the `playwright` option is applied to Chromium only. - Use the `istanbul` coverage provider rather than `v8`. v8 coverage is gathered over CDP, which only Chromium supports, so `coverage` refused to start once the other engines were added. `istanbul` counts default parameters and some conditionals that `v8` did not, which surfaced an untested documented option, so also test `parseBody: false` and `create()`/`extend()` with no overrides. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/main.yaml | 16 +++++++++------- .gitignore | 2 ++ package.json | 2 +- tests/10-client-api.spec.js | 29 +++++++++++++++++++++++++++++ tests/utils.js | 15 +++++++++------ vitest.config.js | 25 +++++++++++++++++++++---- 6 files changed, 71 insertions(+), 18 deletions(-) diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 2494ce7..241e6c5 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -45,7 +45,8 @@ jobs: run: npm run test-node test-browser: runs-on: ubuntu-latest - timeout-minutes: 10 + # three browser engines plus a cold browser install + timeout-minutes: 20 strategy: matrix: node-version: [24.x] @@ -63,13 +64,14 @@ jobs: with: path: ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ hashFiles('package.json') }} - - name: Install Playwright chromium - run: npx playwright install --with-deps chromium + - name: Install Playwright browsers + run: npx playwright install --with-deps chromium firefox webkit - name: Run browser tests run: npm run test-browser coverage: runs-on: ubuntu-latest - timeout-minutes: 10 + # the browser project runs here too, across three engines + timeout-minutes: 20 strategy: matrix: node-version: [24.x] @@ -82,14 +84,14 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: npm install - # coverage runs the browser project too, so a browser is required + # coverage runs the browser project too, so the browsers are required - name: Cache Playwright browsers uses: actions/cache@v4 with: path: ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ hashFiles('package.json') }} - - name: Install Playwright chromium - run: npx playwright install --with-deps chromium + - name: Install Playwright browsers + run: npx playwright install --with-deps chromium firefox webkit - name: Generate coverage report run: npm run coverage-ci - name: Upload coverage to Codecov diff --git a/.gitignore b/.gitignore index cdef007..14675e8 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ *.sw[nop] *~ .cache +.vitest-attachments .nyc_output .project .settings @@ -9,5 +10,6 @@ TAGS coverage dist +__screenshots__ node_modules reports diff --git a/package.json b/package.json index 5c80c4a..c47e3be 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "devDependencies": { "@digitalbazaar/eslint-config": "^9.0.0", "@vitest/browser-playwright": "^4.1.10", - "@vitest/coverage-v8": "^4.1.10", + "@vitest/coverage-istanbul": "^4.1.10", "cors": "^2.8.6", "eslint": "^10.8.1", "express": "^5.2.1", diff --git a/tests/10-client-api.spec.js b/tests/10-client-api.spec.js index f10569e..74761cb 100644 --- a/tests/10-client-api.spec.js +++ b/tests/10-client-api.spec.js @@ -224,6 +224,35 @@ describe('http-client API', () => { response.data.headers.accept.should.equal('text/html'); }); + it('create() and extend() keep the defaults with no overrides', async () => { + const url = `http://${httpHost}/headers`; + for(const client of [httpClient.create({}), httpClient.extend({})]) { + const response = await client.get(url); + response.status.should.equal(200); + response.data.headers.accept.should.equal( + 'application/ld+json, application/json'); + } + }); + + it('does not parse the body when `parseBody` is false', async () => { + let err; + let response; + const url = `http://${httpHost}/json`; + try { + response = await httpClient.get(url, {parseBody: false}); + } catch(e) { + err = e; + } + should.not.exist(err); + should.exist(response); + response.status.should.equal(200); + // `data` is always defined as a property, but left undefined + should.not.exist(response.data); + // the body is untouched, so the caller can still read it + const body = await response.json(); + should.exist(body); + }); + it('proxies the `stop` signal from `ky`', async () => { const stop = await httpClient.stop; should.exist(stop); diff --git a/tests/utils.js b/tests/utils.js index dbb4ed6..1455bdc 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -37,12 +37,15 @@ export async function startServers() { }); await Promise.all([_httpStarted, _httpsStarted]); - const httpServerAddress = httpServer.address(); - const httpsServerAddress = httpsServer.address(); - const httpHost = - `${httpServerAddress.address}:${httpServerAddress.port}`; - const httpsHost = - `${httpsServerAddress.address}:${httpsServerAddress.port}`; + /* + The servers bind to every interface, so `address()` reports `0.0.0.0`. + Only Chromium treats that as loopback when used as a request host; Firefox + and WebKit refuse to connect to it. Advertise the loopback address instead + so the same host works in every engine. + */ + const clientHost = '127.0.0.1'; + const httpHost = `${clientHost}:${httpServer.address().port}`; + const httpsHost = `${clientHost}:${httpsServer.address().port}`; return { httpServer, diff --git a/vitest.config.js b/vitest.config.js index c62be84..6ec9d8b 100644 --- a/vitest.config.js +++ b/vitest.config.js @@ -12,7 +12,10 @@ export default defineConfig({ // `coverage` is process-wide: it can only be set at the root, never // inside a project, and applies across every project in the run coverage: { - provider: 'v8', + // `istanbul` rather than `v8`: v8 coverage is gathered over CDP, which + // only Chromium supports, and the browser project runs in Firefox and + // WebKit as well + provider: 'istanbul', reporter: ['lcov', 'text-summary', 'text'], include: ['lib/**/*.js'] }, @@ -39,11 +42,25 @@ export default defineConfig({ headless: true, provider: playwright({ launchOptions: { - // required to launch Chromium in CI containers - args: ['--no-sandbox', '--disable-setuid-sandbox'] + // needed to launch Chromium in CI containers; `playwright` + // applies this only to Chromium, so it is safe to set for + // every instance (a raw `--no-sandbox` arg is not -- WebKit + // rejects unknown options and fails to launch) + chromiumSandbox: false } }), - instances: [{browser: 'chromium'}] + /* + The `Possible CORS error` message in `lib/httpClient.js` keys off + the browser's own network-error text, which differs per engine, so + each engine has to be exercised to keep that mapping honest. The + shared `10-client-api.spec.js` runs per instance as well, which is + what catches any other behavior difference between engines. + */ + instances: [ + {browser: 'chromium'}, + {browser: 'firefox'}, + {browser: 'webkit'} + ] } } }