Skip to content

Commit c7af01f

Browse files
author
Reflex
committed
Merge branch 'main' into jrvb/h2-transport-integration
# Conflicts: # loadtest/sse-test.ts # src/lib/h2-transport/index.ts # src/lib/h2-transport/pool.ts # src/lib/h2-transport/response.ts # src/lib/h2-transport/session.ts # tests/lib/h2-transport.test.ts
2 parents e6c28d6 + 79f0e6f commit c7af01f

7 files changed

Lines changed: 100 additions & 38 deletions

File tree

loadtest/sse-test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ async function main() {
119119
console.log('PASS');
120120

121121
console.log('\n=== All SSE tests PASSED ===');
122+
await h2Fetch.close();
122123
close();
123124

124125
// Cleanup

src/lib/h2-transport/index.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,27 @@ function checkNodeVersion(): void {
3636

3737
export type { H2PoolOptions as H2FetchOptions };
3838

39-
function normalizeBody(body: unknown): string | Buffer | null {
39+
export type H2Fetch = Fetch & {
40+
close: () => Promise<void>;
41+
};
42+
43+
async function bufferReadable(stream: Readable): Promise<Buffer> {
44+
const chunks: Buffer[] = [];
45+
for await (const chunk of stream) {
46+
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
47+
}
48+
return Buffer.concat(chunks);
49+
}
50+
51+
async function normalizeBody(body: unknown): Promise<string | Buffer | null> {
4052
if (body == null) return null;
4153
if (typeof body === 'string') return body;
4254
if (Buffer.isBuffer(body)) return body;
4355
if (body instanceof MultipartBody) return normalizeBody((body as MultipartBody).body);
4456
if (body instanceof Readable) {
45-
// Multipart bodies arrive as Node Readable. For H2, we need to buffer them
46-
// since session.request() takes string | Buffer. The Readable is a
47-
// FormDataEncoder stream with a known content-length, so buffering is safe.
48-
// Streaming uploads could be added later if needed.
49-
throw new Error(
50-
'h2-transport: streaming request bodies (Readable) are not yet supported. ' +
51-
'Use a string or Buffer body.',
52-
);
57+
// Multipart bodies arrive as a FormDataEncoder Readable with a known
58+
// content-length. session.request() takes string | Buffer, so buffer here.
59+
return bufferReadable(body);
5360
}
5461
if (ArrayBuffer.isView(body)) return Buffer.from(body.buffer, body.byteOffset, body.byteLength);
5562
if (body instanceof ArrayBuffer) return Buffer.from(body);
@@ -62,7 +69,7 @@ function normalizeBody(body: unknown): string | Buffer | null {
6269
* Compatible with the SDK's `makeHttp2Fetch` interface: called with no arguments
6370
* for `http2: true`, or with options for `http2: <H2FetchOptions>`.
6471
*/
65-
export function createH2Fetch(options?: H2PoolOptions): Fetch {
72+
export function createH2Fetch(options?: H2PoolOptions): H2Fetch {
6673
checkNodeVersion();
6774
const pools = new Map<string, H2Pool>();
6875

@@ -75,7 +82,7 @@ export function createH2Fetch(options?: H2PoolOptions): Fetch {
7582
return pool;
7683
}
7784

78-
return async (url, init) => {
85+
const h2Fetch = (async (url, init) => {
7986
const {
8087
agent: _ignored, // node-fetch artifact injected by core.ts
8188
body: rawBody,
@@ -86,7 +93,7 @@ export function createH2Fetch(options?: H2PoolOptions): Fetch {
8693

8794
const parsed = typeof url === 'string' ? new URL(url) : new URL(url.toString());
8895
const path = parsed.pathname + parsed.search;
89-
const body = normalizeBody(rawBody);
96+
const body = await normalizeBody(rawBody);
9097

9198
const reqHeaders: Record<string, string> = {};
9299
if (rawHeaders) {
@@ -103,5 +110,13 @@ export function createH2Fetch(options?: H2PoolOptions): Fetch {
103110

104111
const pool = getPool(parsed.origin);
105112
return pool.request(path, method.toUpperCase(), reqHeaders, body, signal) as any;
113+
}) as H2Fetch;
114+
115+
h2Fetch.close = async () => {
116+
const closeTasks = [...pools.values()].map((pool) => pool.close());
117+
pools.clear();
118+
await Promise.all(closeTasks);
106119
};
120+
121+
return h2Fetch;
107122
}

src/lib/h2-transport/pool.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface H2PoolOptions extends H2SessionOptions {
88

99
const DEFAULT_MIN_CONNECTIONS = 4;
1010
const DEFAULT_MAX_CONNECTIONS = 20;
11+
const RETRYABLE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']);
1112

1213
interface QueuedRequest {
1314
path: string;
@@ -46,9 +47,16 @@ export class H2Pool {
4647
if (this._initialized) return;
4748
if (this._initPromise) return this._initPromise;
4849

49-
this._initPromise = this._createInitialSessions();
50-
await this._initPromise;
51-
this._initialized = true;
50+
const attempt = this._createInitialSessions();
51+
this._initPromise = attempt;
52+
try {
53+
await attempt;
54+
this._initialized = true;
55+
} catch (err) {
56+
// Don't cache the failure — let subsequent requests retry initialization.
57+
if (this._initPromise === attempt) this._initPromise = null;
58+
throw err;
59+
}
5260
}
5361

5462
private async _createInitialSessions(): Promise<void> {
@@ -134,7 +142,7 @@ export class H2Pool {
134142
try {
135143
return await session.request(path, method, headers, body, signal);
136144
} catch (err: any) {
137-
if (session.state !== SessionState.READY) {
145+
if (session.state !== SessionState.READY && RETRYABLE_METHODS.has(method)) {
138146
// Session went away (GOAWAY, etc). Re-enter the pool for a retry.
139147
return this._enqueueRequest(path, method, headers, body, signal);
140148
}
@@ -217,14 +225,14 @@ export class H2Pool {
217225

218226
async close(): Promise<void> {
219227
this._closed = true;
220-
for (const s of this._sessions) {
221-
s.close();
222-
}
228+
const sessions = [...this._sessions];
223229
this._sessions.length = 0;
224230

225231
for (const req of this._queue) {
226232
req.reject(new Error('Pool is closed'));
227233
}
228234
this._queue.length = 0;
235+
236+
await Promise.all(sessions.map((s) => s.close()));
229237
}
230238
}

src/lib/h2-transport/response.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Blob } from 'node:buffer';
12
import { H2Headers } from './headers';
23

34
/**
@@ -53,4 +54,9 @@ export class H2Response {
5354
const buf = await this._consumeBody();
5455
return buf.buffer.slice(buf.byteOffset, buf.byteOffset + buf.byteLength) as ArrayBuffer;
5556
}
57+
58+
async blob(): Promise<Blob> {
59+
const buf = await this._consumeBody();
60+
return new Blob([buf], { type: this.headers.get('content-type') ?? undefined });
61+
}
5662
}

src/lib/h2-transport/session.ts

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ export interface H2SessionOptions {
1919

2020
const DEFAULT_CONNECT_TIMEOUT = 30_000;
2121

22+
const createAbortError = (): Error => Object.assign(new Error('Request aborted'), { name: 'AbortError' });
23+
2224
export class H2Session {
2325
readonly origin: string;
2426

@@ -65,14 +67,16 @@ export class H2Session {
6567

6668
session.on('connect', () => {
6769
clearTimeout(timeout);
68-
const serverMax = session.remoteSettings?.maxConcurrentStreams;
69-
if (serverMax != null && !this._opts.maxConcurrentStreams) {
70-
this._maxConcurrentStreams = serverMax;
71-
}
7270
this._state = SessionState.READY;
7371
resolve();
7472
});
7573

74+
session.on('remoteSettings', (settings) => {
75+
if (settings.maxConcurrentStreams != null && !this._opts.maxConcurrentStreams) {
76+
this._maxConcurrentStreams = settings.maxConcurrentStreams;
77+
}
78+
});
79+
7680
session.on('error', (err) => {
7781
clearTimeout(timeout);
7882
if (this._state === SessionState.CONNECTING) {
@@ -127,10 +131,11 @@ export class H2Session {
127131
this._activeStreams++;
128132

129133
let settled = false;
134+
let cleaned = false;
130135

131136
const onAbort = () => {
132-
if (!settled) {
133-
stream.destroy(new Error('Request aborted'));
137+
if (!cleaned) {
138+
stream.destroy(createAbortError());
134139
}
135140
};
136141

@@ -139,23 +144,27 @@ export class H2Session {
139144
stream.on('error', () => {});
140145
stream.destroy();
141146
this._activeStreams--;
142-
reject(new Error('Request aborted'));
147+
reject(createAbortError());
143148
return;
144149
}
145150
signal.addEventListener('abort', onAbort, { once: true });
146151
}
147152

148153
const cleanup = () => {
154+
if (cleaned) return;
155+
cleaned = true;
149156
this._activeStreams--;
150157
if (signal) signal.removeEventListener('abort', onAbort);
151158
if (this._state === SessionState.DRAINING && this._activeStreams === 0) {
152159
this._close();
153160
}
154-
if (this.available) {
155-
this.onAvailable?.();
156-
}
161+
// Always notify the pool so it can route queued work to other sessions
162+
// (or grow) when this session is draining/closed and can't take more.
163+
this.onAvailable?.();
157164
};
158165

166+
stream.once('close', cleanup);
167+
159168
stream.on('error', (err) => {
160169
if (!settled) {
161170
settled = true;
@@ -206,8 +215,17 @@ export class H2Session {
206215
});
207216
}
208217

209-
close(): void {
210-
this._close();
218+
close(): Promise<void> {
219+
const session = this._session;
220+
if (!session || session.destroyed) {
221+
this._close();
222+
return Promise.resolve();
223+
}
224+
225+
return new Promise((resolve) => {
226+
session.once('close', resolve);
227+
this._close();
228+
});
211229
}
212230

213231
private _close(): void {

src/lib/undici-fetch.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,12 @@ import { MultipartBody } from '../_shims/MultipartBody';
3737
import { type Fetch } from '../core';
3838

3939
const KEEP_ALIVE_TIMEOUT_MS = 10 * 60 * 1000;
40-
// Bound the pool to a few TLS sessions per origin and multiplex many H2 streams
41-
// over each. 4 x 64 = 256 concurrent requests in flight before undici queues the rest.
42-
const H2_MAX_CONNECTIONS = 4;
43-
const H2_MAX_CONCURRENT_STREAMS = 64;
40+
// Bound the pool to several TLS sessions per origin and multiplex H2 streams over
41+
// each. The server typically advertises MAX_CONCURRENT_STREAMS=100-128; we use 128
42+
// per connection to match. 20 x 128 = 2560 concurrent requests in flight before
43+
// undici queues the rest.
44+
const H2_MAX_CONNECTIONS = 20;
45+
const H2_MAX_CONCURRENT_STREAMS = 128;
4446

4547
// One module-scoped default dispatcher, reused across requests: a bounded HTTP/2 pool
4648
// with keep-alive. `allowH2` negotiates h2 over TLS via ALPN and transparently falls

tests/lib/h2-transport.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ function startTestServer(
3636
handler: (stream: http2.ServerHttp2Stream, headers: http2.IncomingHttpHeaders) => void,
3737
): Promise<{ port: number; close: () => Promise<void> }> {
3838
return new Promise((resolve) => {
39+
const sessions = new Set<http2.ServerHttp2Session>();
3940
const server = http2.createSecureServer({
4041
key: fs.readFileSync(keyPath),
4142
cert: fs.readFileSync(certPath),
@@ -44,7 +45,6 @@ function startTestServer(
4445
stream.on('error', () => {});
4546
handler(stream, headers);
4647
});
47-
const sessions = new Set<http2.ServerHttp2Session>();
4848
server.on('session', (session) => {
4949
sessions.add(session);
5050
session.on('error', () => {});
@@ -56,8 +56,7 @@ function startTestServer(
5656
port,
5757
close: () =>
5858
new Promise<void>((res) => {
59-
for (const session of sessions) session.destroy();
60-
sessions.clear();
59+
for (const session of sessions) session.close();
6160
server.close(() => res());
6261
}),
6362
});
@@ -464,4 +463,17 @@ describe('normalizeBody', () => {
464463
const body = await resp.json();
465464
expect(body.echoed).toBe('buffer data');
466465
});
466+
467+
test('Readable body is buffered', async () => {
468+
const { Readable } = await import('node:stream');
469+
const h2Fetch = createH2Fetch({ tlsOptions: testTls });
470+
const stream = Readable.from([Buffer.from('chunk-1|'), Buffer.from('chunk-2')]);
471+
const resp = (await h2Fetch(`https://localhost:${server.port}/echo`, {
472+
method: 'POST',
473+
headers: {},
474+
body: stream,
475+
})) as any;
476+
const body = await resp.json();
477+
expect(body.echoed).toBe('chunk-1|chunk-2');
478+
});
467479
});

0 commit comments

Comments
 (0)