Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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
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.

2 changes: 1 addition & 1 deletion 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` */
/** Allow negotiating HTTP/2 with capable servers via ALPN. Since undici@8 this is enabled by default. */
allowH2?: boolean;
Comment thread
fengmk2 marked this conversation as resolved.
Outdated
/** Custom DNS lookup function, default is `dns.lookup`. */
lookup?: LookupFunction;
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
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ interface UrllibRequestOptions extends RequestOptions {
* verification fails. Default: `true`
*/
rejectUnauthorized?: boolean;
/** Allow to use HTTP2 first. Default is `false` */
/** Allow negotiating HTTP/2 with capable servers via ALPN. Since undici@8 this is enabled by default. */
allowH2?: boolean;
Comment thread
fengmk2 marked this conversation as resolved.
Outdated
}

Expand Down
4 changes: 3 additions & 1 deletion test/HttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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
5 changes: 5 additions & 0 deletions test/options.dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ describe('options.dispatcher.test.ts', () => {
let _url: string;
let proxyServer: any;
let proxyServerUrl: string;
const proxyAgents: ProxyAgent[] = [];
beforeAll(async () => {
const { closeServer, url } = await startServer();
close = closeServer;
Expand All @@ -27,13 +28,16 @@ describe('options.dispatcher.test.ts', () => {

afterAll(async () => {
await close();
// close keep-alive proxy connections so the proxy server can shut down
await Promise.all(proxyAgents.map((proxyAgent) => proxyAgent.close()));
await new Promise((resolve) => {
proxyServer.close(resolve);
});
});

it('should work with proxyAgent dispatcher', async () => {
const proxyAgent = new ProxyAgent(proxyServerUrl);
proxyAgents.push(proxyAgent);
const response = await request(`${_url}html`, {
dispatcher: proxyAgent,
dataType: 'text',
Expand All @@ -55,6 +59,7 @@ describe('options.dispatcher.test.ts', () => {
it('should work with getGlobalDispatcher() dispatcher', async () => {
const agent = getGlobalDispatcher();
const proxyAgent = new ProxyAgent(proxyServerUrl);
proxyAgents.push(proxyAgent);
setGlobalDispatcher(proxyAgent);
const response = await request(`${_url}html`, {
dataType: 'text',
Expand Down
7 changes: 4 additions & 3 deletions test/options.timeout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ describe('options.timeout.test.ts', () => {
// console.error(err);
assert.equal(err.name, 'HttpClientRequestTimeoutError');
assert.equal(err.message, 'Request timeout for 10 ms');
assert.equal(err.cause.name, 'InformationalError');
assert.equal(err.cause.message, 'HTTP/2: "stream timeout after 10"');
assert.equal(err.cause.code, 'UND_ERR_INFO');
// undici@8 reports HTTP/2 headers timeout as a standard HeadersTimeoutError
assert.equal(err.cause.name, 'HeadersTimeoutError');
assert.equal(err.cause.message, 'HTTP/2: "headers timeout after 10"');
assert.equal(err.cause.code, 'UND_ERR_HEADERS_TIMEOUT');

assert.equal(err.res.status, -1);
assert(err.res.rt > 10, `actual ${err.res.rt}`);
Expand Down
Loading