Skip to content

Commit b87b790

Browse files
Reflexclaude
andcommitted
refactor(http2): extract transport selection to src/lib and share the default pool
Addresses PR review: - Move the HTTP/2 transport-selection logic and one-time warnings out of the generated `src/index.ts` into a handwritten `src/lib/http2-transport.ts` helper (`resolveHttp2Fetch`). The generated file keeps only a one-line call-site hook, following the existing `makeHttp2Fetch` / `H2FetchOptions` pattern, so regeneration stays clean. - Share a single default HTTP/2 pool across all clients instead of building a new pool per `new Runloop()`. Short-lived clients no longer leak H2 sessions. Explicit `H2FetchOptions` still get a dedicated pool (intentional tuning). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e4bbb25 commit b87b790

3 files changed

Lines changed: 131 additions & 55 deletions

File tree

src/index.ts

Lines changed: 5 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// File generated from our OpenAPI spec by Stainless. See CONTRIBUTING.md for details.
22

3-
import { type Agent, makeHttp2Fetch } from './_shims/index';
3+
import { type Agent } from './_shims/index';
4+
import { resolveHttp2Fetch } from './lib/http2-transport';
45
import * as Core from './core';
56
import * as Errors from './error';
67
import * as Pagination from './pagination';
@@ -347,16 +348,6 @@ export interface ClientOptions {
347348
* console.log(result.exitCode);
348349
* ```
349350
*/
350-
// Emitted at most once per process each (see the constructor). Module-scoped flags
351-
// mirror the `fileFromPathWarned` pattern in _shims/node-runtime.ts.
352-
//
353-
// `http2HttpAgentWarned` — user explicitly set `http2` *and* `httpAgent`; the agent
354-
// is ignored on the H2 path.
355-
// `http2DefaultAgentWarned`— user set `httpAgent` without an explicit `http2` value, so
356-
// the client stays on HTTP/1.1 rather than the H2 default.
357-
let http2HttpAgentWarned = false;
358-
let http2DefaultAgentWarned = false;
359-
360351
export class Runloop extends Core.APIClient {
361352
bearerToken: string;
362353

@@ -392,56 +383,15 @@ export class Runloop extends Core.APIClient {
392383
baseURL: baseURL || `https://api.runloop.ai`,
393384
};
394385

395-
// Resolve the transport. HTTP/2 is the default on Node; a user-supplied
396-
// `fetch` always wins, `http2: false` opts out, and a bare `httpAgent`
397-
// (no explicit `http2`) keeps the client on HTTP/1.1 so existing agents
398-
// keep working. `httpAgent` never applies to the H2 path — it manages its
399-
// own persistent connections.
400-
let useHttp2: boolean;
401-
if (options.fetch) {
402-
// Custom fetch owns the transport entirely.
403-
useHttp2 = false;
404-
} else if (options.http2 === false) {
405-
useHttp2 = false;
406-
} else if (options.http2) {
407-
// Explicit opt-in (`true` or H2FetchOptions).
408-
useHttp2 = true;
409-
if (options.httpAgent && !http2HttpAgentWarned) {
410-
http2HttpAgentWarned = true;
411-
console.warn(
412-
'[runloop] `httpAgent` is ignored when `http2` is set: the HTTP/2 transport manages ' +
413-
'its own connections. To tune the H2 pool, pass options as `http2` ' +
414-
'(e.g. `http2: { maxConnections: 20 }`).',
415-
);
416-
}
417-
} else if (options.httpAgent) {
418-
// Default would be H2, but a `httpAgent` was provided — respect it and
419-
// stay on HTTP/1.1, warning once so the fallback isn't silent.
420-
useHttp2 = false;
421-
if (!http2DefaultAgentWarned) {
422-
http2DefaultAgentWarned = true;
423-
console.warn(
424-
'[runloop] HTTP/2 is the default transport, but `httpAgent` is set, so this client ' +
425-
'uses HTTP/1.1. Remove `httpAgent` to use HTTP/2, or pass `http2: false` to select ' +
426-
'HTTP/1.1 explicitly and silence this warning.',
427-
);
428-
}
429-
} else {
430-
// New default: HTTP/2.
431-
useHttp2 = true;
432-
}
433-
434386
super({
435387
baseURL: options.baseURL!,
436388
baseURLOverridden: baseURL ? baseURL !== 'https://api.runloop.ai' : false,
437389
timeout: options.timeout ?? 30000 /* 30 seconds */,
438390
httpAgent: options.httpAgent,
439391
maxRetries: options.maxRetries,
440-
fetch:
441-
options.fetch ??
442-
(useHttp2 ?
443-
makeHttp2Fetch(typeof options.http2 === 'object' ? options.http2 : undefined)
444-
: undefined),
392+
// HTTP/2 is the default transport on Node; a custom `fetch` always wins.
393+
// See `resolveHttp2Fetch` for the `http2` / `httpAgent` resolution.
394+
fetch: options.fetch ?? resolveHttp2Fetch(options),
445395
});
446396

447397
const customHeadersEnv = Core.readEnv('RUNLOOP_CUSTOM_HEADERS');

src/lib/http2-transport.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { makeHttp2Fetch } from '../_shims/index';
2+
import { type Fetch } from '../core';
3+
import type { H2FetchOptions } from './h2-transport';
4+
5+
/**
6+
* Transport-selection logic for the client. Kept out of the generated
7+
* `src/index.ts` so the generated file only holds a one-line call-site hook.
8+
*
9+
* HTTP/2 is the default on Node. The web/Deno shims make {@link makeHttp2Fetch}
10+
* a no-op that returns the platform `fetch`, so this is effectively Node-only.
11+
*/
12+
13+
// A single HTTP/2 transport shared by every client that uses the default pool,
14+
// so short-lived clients don't each open (and leak) their own H2 sessions.
15+
// Clients that pass explicit `H2FetchOptions` get a dedicated pool, since they
16+
// asked to tune it.
17+
let sharedDefaultFetch: Fetch | undefined;
18+
19+
function getSharedDefaultFetch(): Fetch {
20+
return (sharedDefaultFetch ??= makeHttp2Fetch());
21+
}
22+
23+
// Each warning is emitted at most once per process. Module-scoped flags mirror
24+
// the `fileFromPathWarned` pattern in _shims/node-runtime.ts.
25+
let httpAgentIgnoredWarned = false;
26+
let httpAgentFallbackWarned = false;
27+
28+
function warnHttpAgentIgnored(): void {
29+
if (httpAgentIgnoredWarned) return;
30+
httpAgentIgnoredWarned = true;
31+
console.warn(
32+
'[runloop] `httpAgent` is ignored when `http2` is set: the HTTP/2 transport manages ' +
33+
'its own connections. To tune the H2 pool, pass options as `http2` ' +
34+
'(e.g. `http2: { maxConnections: 20 }`).',
35+
);
36+
}
37+
38+
function warnHttpAgentFallback(): void {
39+
if (httpAgentFallbackWarned) return;
40+
httpAgentFallbackWarned = true;
41+
console.warn(
42+
'[runloop] HTTP/2 is the default transport, but `httpAgent` is set, so this client ' +
43+
'uses HTTP/1.1. Remove `httpAgent` to use HTTP/2, or pass `http2: false` to select ' +
44+
'HTTP/1.1 explicitly and silence this warning.',
45+
);
46+
}
47+
48+
export interface Http2TransportOptions {
49+
http2?: boolean | H2FetchOptions | undefined;
50+
httpAgent?: unknown;
51+
}
52+
53+
/**
54+
* Resolve the `fetch` implementation for the transport, or `undefined` to fall
55+
* back to the default HTTP/1.1 (`node-fetch`) transport. Only called when the
56+
* caller did not supply a custom `fetch` (a custom `fetch` always wins).
57+
*
58+
* - `http2: false` → HTTP/1.1.
59+
* - `http2: true` / omitted → the shared default HTTP/2 pool.
60+
* - `http2: H2FetchOptions` → a dedicated HTTP/2 pool tuned with those options.
61+
* - a bare `httpAgent` (no explicit `http2`) → HTTP/1.1, so existing agents keep
62+
* working; warns once since it overrides the HTTP/2 default.
63+
*/
64+
export function resolveHttp2Fetch(options: Http2TransportOptions): Fetch | undefined {
65+
if (options.http2 === false) return undefined;
66+
67+
if (options.http2) {
68+
if (options.httpAgent) warnHttpAgentIgnored();
69+
return typeof options.http2 === 'object' ? makeHttp2Fetch(options.http2) : getSharedDefaultFetch();
70+
}
71+
72+
if (options.httpAgent) {
73+
warnHttpAgentFallback();
74+
return undefined;
75+
}
76+
77+
return getSharedDefaultFetch();
78+
}

tests/lib/http2-transport.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { resolveHttp2Fetch } from '../../src/lib/http2-transport';
2+
3+
describe('resolveHttp2Fetch', () => {
4+
test('defaults to HTTP/2 and shares a single pool across clients', () => {
5+
const a = resolveHttp2Fetch({});
6+
const b = resolveHttp2Fetch({});
7+
const explicitTrue = resolveHttp2Fetch({ http2: true });
8+
expect(a).toBeDefined();
9+
// Same instance — short-lived clients must not each open their own pool.
10+
expect(b).toBe(a);
11+
expect(explicitTrue).toBe(a);
12+
});
13+
14+
test('http2: false falls back to HTTP/1.1 (undefined fetch)', () => {
15+
expect(resolveHttp2Fetch({ http2: false })).toBeUndefined();
16+
});
17+
18+
test('H2FetchOptions gets a dedicated pool, not the shared default', () => {
19+
const shared = resolveHttp2Fetch({});
20+
const tuned = resolveHttp2Fetch({ http2: { maxConnections: 2 } });
21+
expect(tuned).toBeDefined();
22+
expect(tuned).not.toBe(shared);
23+
});
24+
25+
test('bare httpAgent keeps HTTP/1.1 and warns once', () => {
26+
const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
27+
try {
28+
expect(resolveHttp2Fetch({ httpAgent: {} })).toBeUndefined();
29+
expect(resolveHttp2Fetch({ httpAgent: {} })).toBeUndefined();
30+
expect(warn).toHaveBeenCalledTimes(1);
31+
expect(String(warn.mock.calls[0]?.[0])).toContain('HTTP/1.1');
32+
} finally {
33+
warn.mockRestore();
34+
}
35+
});
36+
37+
test('http2 + httpAgent uses H2 and warns that httpAgent is ignored', () => {
38+
const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
39+
try {
40+
const shared = resolveHttp2Fetch({});
41+
expect(resolveHttp2Fetch({ http2: true, httpAgent: {} })).toBe(shared);
42+
expect(warn).toHaveBeenCalledTimes(1);
43+
expect(String(warn.mock.calls[0]?.[0])).toContain('httpAgent');
44+
} finally {
45+
warn.mockRestore();
46+
}
47+
});
48+
});

0 commit comments

Comments
 (0)