Skip to content

Commit fd815df

Browse files
committed
fix(api): always use node:https.request in apiFetch to avoid undici body timeout
Node's built-in fetch uses undici which has a default bodyTimeout of 300s. For large streaming ND-JSON responses (e.g. full scan results) this timeout fires before the body has fully transferred, producing a BodyTimeoutError. Route all apiFetch calls through the existing _httpsRequestFetch path, which uses node:https.request and has no body timeout — consistent with the SDK and all other HTTP clients used across socket-cli, socket-sdk-js, and coana. When SSL_CERT_FILE is not set, agent is passed as undefined and https.request uses its default agent. Behaviour is otherwise identical to before. Update the apiFetch test to assert https.request is always used and that no custom HttpsAgent is created when CA certs are not configured.
1 parent bb561fa commit fd815df

2 files changed

Lines changed: 47 additions & 19 deletions

File tree

src/utils/api.mts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,12 @@ function getHttpsAgent(): HttpsAgent | undefined {
7272
return _httpsAgent
7373
}
7474

75-
// Wrapper around fetch that supports extra CA certificates via SSL_CERT_FILE.
76-
// Uses node:https.request with a custom agent when extra CA certs are needed,
77-
// falling back to regular fetch() otherwise. Follows redirects like fetch().
75+
// Wrapper around fetch using node:https.request for all calls.
76+
// This avoids undici's default 300 s bodyTimeout, which would otherwise fire
77+
// on large streaming responses before the body has fully transferred. Behaviour
78+
// matches the SDK, which also uses node:https.request with no body timeout.
79+
// Passes the custom HTTPS agent when extra CA certificates are configured via
80+
// SSL_CERT_FILE, otherwise uses Node's default agent. Follows redirects like fetch().
7881
export type ApiFetchInit = {
7982
body?: string | undefined
8083
headers?: Record<string, string> | undefined
@@ -85,7 +88,7 @@ export type ApiFetchInit = {
8588
function _httpsRequestFetch(
8689
url: string,
8790
init: ApiFetchInit,
88-
agent: HttpsAgent,
91+
agent: HttpsAgent | undefined,
8992
redirectCount: number,
9093
): Promise<Response> {
9194
return new Promise((resolve, reject) => {
@@ -186,11 +189,7 @@ export async function apiFetch(
186189
url: string,
187190
init: ApiFetchInit = {},
188191
): Promise<Response> {
189-
const agent = getHttpsAgent()
190-
if (!agent) {
191-
return await fetch(url, init as globalThis.RequestInit)
192-
}
193-
return await _httpsRequestFetch(url, init, agent, 0)
192+
return await _httpsRequestFetch(url, init, getHttpsAgent(), 0)
194193
}
195194

196195
export type CommandRequirements = {

src/utils/api.test.mts

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
* direct API calls when NODE_EXTRA_CA_CERTS is not set at process startup.
77
*
88
* Test Coverage:
9-
* - apiFetch falls back to regular fetch when no extra CA certs are needed.
10-
* - apiFetch uses node:https.request with custom agent when CA certs are set.
9+
* - apiFetch always uses node:https.request (no undici body timeout).
10+
* - apiFetch passes a custom HttpsAgent when CA certs are set via SSL_CERT_FILE.
11+
* - apiFetch passes no agent (undefined) when no CA certs are configured.
1112
* - Response object construction from https.request output.
1213
* - POST requests with JSON body through https.request path.
1314
* - Error propagation from https.request failures.
@@ -113,18 +114,46 @@ describe('apiFetch with extra CA certificates', () => {
113114
globalThis.fetch = originalFetch
114115
})
115116

116-
it('should use regular fetch when no extra CA certs are needed', async () => {
117-
const mockResponse = new Response(JSON.stringify({ ok: true }), {
118-
status: 200,
119-
statusText: 'OK',
120-
})
121-
globalThis.fetch = vi.fn().mockResolvedValue(mockResponse)
117+
it('should use https.request with no agent when no extra CA certs are needed', async () => {
118+
const mockReq = {
119+
end: vi.fn(),
120+
on: vi.fn(),
121+
write: vi.fn(),
122+
}
123+
124+
mockHttpsRequest.mockImplementation(
125+
(_url: string, _opts: unknown, callback: RequestCallback) => {
126+
setTimeout(() => {
127+
const mockRes = {
128+
headers: { 'content-type': 'text/plain' },
129+
on: vi.fn(),
130+
statusCode: 200,
131+
statusMessage: 'OK',
132+
}
133+
const handlers: Record<string, Function> = {}
134+
mockRes.on.mockImplementation((event: string, handler: Function) => {
135+
handlers[event] = handler
136+
return mockRes
137+
})
138+
callback(mockRes)
139+
handlers['data']?.(Buffer.from('response body'))
140+
handlers['end']?.()
141+
}, 0)
142+
return mockReq
143+
},
144+
)
122145

123146
const { queryApiSafeText } = await import('./api.mts')
124147
const result = await queryApiSafeText('test/path', 'test request')
125148

126-
expect(globalThis.fetch).toHaveBeenCalled()
127-
expect(mockHttpsRequest).not.toHaveBeenCalled()
149+
// Always uses https.request — no undici body timeout.
150+
expect(mockHttpsRequest).toHaveBeenCalled()
151+
// No custom HttpsAgent created when CA certs are not configured.
152+
expect(MockHttpsAgent).not.toHaveBeenCalled()
153+
// agent is undefined when no CA certs are configured.
154+
const callArgs = mockHttpsRequest.mock.calls[0]
155+
expect(callArgs[1]).toEqual(expect.objectContaining({ agent: undefined }))
156+
expect(result.ok).toBe(true)
128157
})
129158

130159
it('should use https.request when extra CA certs are available', async () => {

0 commit comments

Comments
 (0)