Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions .claude/settings.json

This file was deleted.

1 change: 0 additions & 1 deletion .claude/skills/vite-plus

This file was deleted.

2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
fail-fast: false
matrix:
os: ['ubuntu-latest', 'macos-latest', 'windows-latest']
node: ['22', '24', '25']
node: ['22', '24', '26']

name: Test (${{ matrix.os }}, ${{ matrix.node }})
runs-on: ${{ matrix.os }}
Expand Down
14 changes: 14 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Patch coverage stays strict (new/changed lines must be covered).
# Project coverage tolerates small fluctuations from external-network tests
# and undici behaviour changes (e.g. an error branch reachable only on a
# different protocol path).
coverage:
status:
project:
default:
target: auto
threshold: 1%
patch:
default:
target: auto
threshold: 0%
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"mime-types": "^2.1.35",
"qs": "^6.15.0",
"type-fest": "^4.41.0",
"undici": "^7.24.0",
"undici": "^8.4.1",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor allowH2:false in FetchFactory options

With this upgrade, omitting allowH2 now means HTTP/2 is enabled by default, but the same ClientOptions type is also used by FetchFactory.setClientOptions(): when callers pass only { allowH2: false }, src/fetch.ts still checks clientOptions?.allowH2 truthily and constructs a BaseAgent without the option, so undici v8 negotiates HTTP/2 anyway. This leaves fetch users without the documented HTTP/1.1 opt-out unless they also provide connect/lookup or a custom dispatcher.

Useful? React with 👍 / 👎.

"ylru": "^2.0.0"
},
"devDependencies": {
Expand Down Expand Up @@ -110,7 +110,7 @@
}
},
"engines": {
"node": ">= 22.0.0"
"node": ">= 22.19.0"
},
"packageManager": "pnpm@11.6.0"
}
32 changes: 16 additions & 16 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/HttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const debug = debuglog('urllib:HttpClient');

export type ClientOptions = {
defaultArgs?: RequestOptions;
/** Allow to use HTTP2 first. Default is `false` */
/** Negotiate HTTP/2 with capable servers via ALPN. Enabled by default since undici@8; set `false` to force HTTP/1.1. */
allowH2?: boolean;
/** Custom DNS lookup function, default is `dns.lookup`. */
lookup?: LookupFunction;
Expand Down Expand Up @@ -205,8 +205,9 @@ export class HttpClient extends EventEmitter {
connect: clientOptions.connect,
allowH2: clientOptions.allowH2,
});
} else if (clientOptions?.allowH2) {
// Support HTTP2
} else if (clientOptions?.allowH2 !== undefined) {
// Pin the protocol when allowH2 is set explicitly: `true` enables HTTP/2,
// `false` forces HTTP/1.1 instead of following undici@8's HTTP/2 default.
this.#dispatcher = new Agent({
allowH2: clientOptions.allowH2,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize stats keys for HTTP/1-only agents

Passing allowH2: false here makes undici v8 store the internal client under a key like https://host#http1-only; getDispatcherPoolStats() exposes those internal kClients keys as documented origins, so callers using new HttpClient({ allowH2: false }) or request(..., { allowH2: false }) can no longer look up stats by the origin URL and get undefined. Consider normalizing that suffix (or using the dispatcher origin) when building the stats map.

Useful? React with 👍 / 👎.

});
Expand Down
2 changes: 1 addition & 1 deletion src/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export type RequestURL = string | URL;
export type FixJSONCtlCharsHandler = (data: string) => string;
export type FixJSONCtlChars = boolean | FixJSONCtlCharsHandler;

type AbortSignal = unknown;
type AbortSignal = globalThis.AbortSignal;

export type RequestOptions = {
/** Request method, defaults to GET. Could be GET, POST, DELETE or PUT. Alias 'type'. */
Expand Down
29 changes: 26 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import type { HttpClientResponse } from './Response.js';

let httpClient: HttpClient;
let allowH2HttpClient: HttpClient;
let disallowH2HttpClient: HttpClient;
let allowUnauthorizedHttpClient: HttpClient;
let allowH2AndUnauthorizedHttpClient: HttpClient;
let disallowH2AndUnauthorizedHttpClient: HttpClient;
const domainSocketHttpClients = new LRU(50);

export function getDefaultHttpClient(rejectUnauthorized?: boolean, allowH2?: boolean): HttpClient {
if (rejectUnauthorized === false) {
if (allowH2) {
if (allowH2 === true) {
if (!allowH2AndUnauthorizedHttpClient) {
allowH2AndUnauthorizedHttpClient = new HttpClient({
allowH2,
Expand All @@ -24,6 +26,18 @@ export function getDefaultHttpClient(rejectUnauthorized?: boolean, allowH2?: boo
return allowH2AndUnauthorizedHttpClient;
}

if (allowH2 === false) {
if (!disallowH2AndUnauthorizedHttpClient) {
disallowH2AndUnauthorizedHttpClient = new HttpClient({
allowH2,
connect: {
rejectUnauthorized,
},
});
}
return disallowH2AndUnauthorizedHttpClient;
}

if (!allowUnauthorizedHttpClient) {
allowUnauthorizedHttpClient = new HttpClient({
connect: {
Expand All @@ -34,7 +48,7 @@ export function getDefaultHttpClient(rejectUnauthorized?: boolean, allowH2?: boo
return allowUnauthorizedHttpClient;
}

if (allowH2) {
if (allowH2 === true) {
if (!allowH2HttpClient) {
allowH2HttpClient = new HttpClient({
allowH2,
Expand All @@ -43,6 +57,15 @@ export function getDefaultHttpClient(rejectUnauthorized?: boolean, allowH2?: boo
return allowH2HttpClient;
}

if (allowH2 === false) {
if (!disallowH2HttpClient) {
disallowH2HttpClient = new HttpClient({
allowH2,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve global dispatchers when disabling H2

In applications that install a global dispatcher with setGlobalDispatcher() (for example a ProxyAgent or MockAgent), request(url, { allowH2: false }) now takes this new branch and creates urllib's own Agent, so the request no longer goes through the configured global proxy/mock; before this change the falsy option fell through to the default client/global dispatcher. Consider applying the HTTP/1-only choice without bypassing the active dispatcher, or requiring callers to pass an explicit dispatcher for this combination.

Useful? React with 👍 / 👎.

}
return disallowH2HttpClient;
}

if (!httpClient) {
httpClient = new HttpClient();
}
Expand All @@ -55,7 +78,7 @@ interface UrllibRequestOptions extends RequestOptions {
* verification fails. Default: `true`
*/
rejectUnauthorized?: boolean;
/** Allow to use HTTP2 first. Default is `false` */
/** Negotiate HTTP/2 with capable servers via ALPN. Enabled by default since undici@8; set `false` to force HTTP/1.1. */
allowH2?: boolean;
}

Expand Down
106 changes: 104 additions & 2 deletions test/HttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import { strict as assert } from 'node:assert';
import dns from 'node:dns';
import { once } from 'node:events';
import { sensitiveHeaders, createSecureServer } from 'node:http2';
import { createServer as createSecureHttp1Server } from 'node:https';
import type { AddressInfo } from 'node:net';
import { PerformanceObserver } from 'node:perf_hooks';
import { setTimeout as sleep } from 'node:timers/promises';

import selfsigned from 'selfsigned';
import { describe, it, beforeAll, afterAll } from 'vite-plus/test';

import { HttpClient, getGlobalDispatcher } from '../src/index.js';
import { HttpClient, getDefaultHttpClient, getGlobalDispatcher } from '../src/index.js';
import type { RawResponseWithMeta } from '../src/index.js';
import { startServer } from './fixtures/server.js';

Expand Down Expand Up @@ -121,7 +122,9 @@ describe('HttpClient.test.ts', () => {
httpClient.request(_url),
]);
// console.log(httpClient.getDispatcherPoolStats());
assert.equal(httpClient.getDispatcherPoolStats()['https://registry.npmmirror.com'].connected, 4);
// undici@8 negotiates HTTP/2 with h2-capable servers, so all requests are
// multiplexed over a single connection instead of opening one per request.
assert.equal(httpClient.getDispatcherPoolStats()['https://registry.npmmirror.com'].connected, 1);
assert(httpClient.getDispatcherPoolStats()[_url.substring(0, _url.length - 1)].connected > 1);
Comment on lines +158 to 161

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test should work with allowH2 = true creates httpClient1 with allowH2: false but never asserts that HTTP/2 is actually disabled for it (it only asserts response.status === 200).

Because of the bug in HttpClient's constructor and getDefaultHttpClient where allowH2: false is ignored, httpClient1 is actually still using HTTP/2 under the hood.

We should add an assertion to verify that httpClient1 indeed uses HTTP/1.1 (e.g., by checking that it does not multiplex requests or by checking the socket/protocol if available) to prevent this regression.

});

Expand Down Expand Up @@ -247,6 +250,105 @@ describe('HttpClient.test.ts', () => {
});
});

describe('protocol negotiation', () => {
it('should use HTTP/1.1 when the server only supports HTTP/1.1', async () => {
const server = createSecureHttp1Server(
{
key: pems.private,
cert: pems.cert,
},
(req, res) => {
res.writeHead(200, { 'content-type': 'text/plain; charset=utf-8' });
res.end(`hello http/${req.httpVersion}!`);
},
);
server.listen(0);
await once(server, 'listening');
const url = `https://localhost:${(server.address() as AddressInfo).port}`;

const httpClient = new HttpClient({
connect: { rejectUnauthorized: false },
});
try {
const response = await httpClient.request<string>(url, { dataType: 'text' });
assert.equal(response.status, 200);
assert.equal(response.data, 'hello http/1.1!');
} finally {
await httpClient.getDispatcher().close();
await new Promise<void>((resolve) => server.close(() => resolve()));
}
});

it('should negotiate HTTP/2 by default when the server supports it', async () => {
// undici@8 enables allowH2 by default, so urllib negotiates HTTP/2 via ALPN.
const server = createSecureServer({
allowHTTP1: true,
key: pems.private,
cert: pems.cert,
});
server.on('request', (req, res) => {
res.writeHead(200, { 'content-type': 'text/plain; charset=utf-8' });
res.end(`hello http/${req.httpVersion}!`);
});
server.listen(0);
await once(server, 'listening');
const url = `https://localhost:${(server.address() as AddressInfo).port}`;

const httpClient = new HttpClient({
connect: { rejectUnauthorized: false },
});
try {
const response = await httpClient.request<string>(url, { dataType: 'text' });
assert.equal(response.status, 200);
assert.equal(response.data, 'hello http/2.0!');
} finally {
await httpClient.getDispatcher().close();
await new Promise<void>((resolve) => server.close(() => resolve()));
}
});

it('should force HTTP/1.1 with allowH2 = false even if the server supports HTTP/2', async () => {
const server = createSecureServer({
allowHTTP1: true,
key: pems.private,
cert: pems.cert,
});
server.on('request', (req, res) => {
res.writeHead(200, { 'content-type': 'text/plain; charset=utf-8' });
res.end(`hello http/${req.httpVersion}!`);
});
server.listen(0);
await once(server, 'listening');
const url = `https://localhost:${(server.address() as AddressInfo).port}`;

const httpClient = new HttpClient({
allowH2: false,
connect: { rejectUnauthorized: false },
});
try {
const response = await httpClient.request<string>(url, { dataType: 'text' });
assert.equal(response.status, 200);
assert.equal(response.data, 'hello http/1.1!');
} finally {
await httpClient.getDispatcher().close();
await new Promise<void>((resolve) => server.close(() => resolve()));
}
});

it('should pin a dedicated dispatcher when allowH2 is set explicitly', () => {
// allowH2: false must not fall back to undici@8's HTTP/2-enabled global dispatcher
assert.notEqual(new HttpClient({ allowH2: false }).getDispatcher(), getGlobalDispatcher());
assert.notEqual(new HttpClient({ allowH2: true }).getDispatcher(), getGlobalDispatcher());
});

it('should cache distinct default clients per allowH2 value', () => {
assert.equal(getDefaultHttpClient(undefined, false), getDefaultHttpClient(undefined, false));
assert.notEqual(getDefaultHttpClient(undefined, false), getDefaultHttpClient(undefined, true));
assert.notEqual(getDefaultHttpClient(undefined, false), getDefaultHttpClient(undefined, undefined));
assert.notEqual(getDefaultHttpClient(false, false), getDefaultHttpClient(false, true));
});
});

describe('clientOptions.defaultArgs', () => {
it('should work with custom defaultArgs', async () => {
const httpclient = new HttpClient({ defaultArgs: { timeout: 1000 } });
Expand Down
Loading
Loading