Skip to content

Commit a3b9ef1

Browse files
Merge pull request #269 from modelstudioai/fix/http-native-fetch-timeouts
fix(http): align Node and Bun timeouts and response cleanup
2 parents 01cbe77 + c05ec96 commit a3b9ef1

9 files changed

Lines changed: 627 additions & 31 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ jobs:
2323
node-version: "22"
2424
cache: pnpm
2525

26+
# Keep native-fetch and compiled Binary tests on the release Bun version.
27+
- uses: oven-sh/setup-bun@v2
28+
with:
29+
bun-version: "1.3.14"
30+
2631
- run: pnpm install --frozen-lockfile
2732

2833
- run: pnpm run sync:skill-assets

‎packages/commands/src/commands/sandbox/instance.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,8 @@ export const sandboxPause = defineCommand({
383383
emitResult({ method: "POST", endpoint, request: null }, format);
384384
return;
385385
}
386-
await ctx.client.request({ path: endpoint, method: "POST" });
386+
const response = await ctx.client.request({ path: endpoint, method: "POST" });
387+
await response.body?.cancel();
387388
if (ctx.settings.quiet) emitBare(ctx.flags.sandboxId);
388389
else emitResult({ sandboxID: ctx.flags.sandboxId, paused: true }, format);
389390
},
@@ -410,7 +411,8 @@ export const sandboxDelete = defineCommand({
410411
emitResult({ method: "DELETE", endpoint, request: null }, format);
411412
return;
412413
}
413-
await ctx.client.request({ path: endpoint, method: "DELETE" });
414+
const response = await ctx.client.request({ path: endpoint, method: "DELETE" });
415+
await response.body?.cancel();
414416
if (ctx.settings.quiet) emitBare(ctx.flags.sandboxId);
415417
else emitResult({ sandboxID: ctx.flags.sandboxId, deleted: true }, format);
416418
},

‎packages/commands/src/commands/sandbox/template.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,8 @@ export const sandboxTemplateDelete = defineCommand({
644644
emitResult({ method: "DELETE", endpoint, request: null }, format);
645645
return;
646646
}
647-
await ctx.client.request({ path: endpoint, method: "DELETE" });
647+
const response = await ctx.client.request({ path: endpoint, method: "DELETE" });
648+
await response.body?.cancel();
648649
if (ctx.settings.quiet) emitBare(ctx.flags.templateId);
649650
else emitResult({ templateID: ctx.flags.templateId, deleted: true }, format);
650651
},

‎packages/commands/tests/sandbox.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { getEventListeners } from "node:events";
12
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
23
import { tmpdir } from "node:os";
34
import { join } from "node:path";
@@ -640,3 +641,52 @@ test("Sandbox validation errors use CLI usage exit codes", async () => {
640641
BailianError,
641642
);
642643
});
644+
645+
describe("Sandbox unused response cleanup", () => {
646+
test.each([
647+
{ name: "pause instance", command: sandboxPause, flags: { sandboxId: "sandbox-test" } },
648+
{ name: "delete instance", command: sandboxDelete, flags: { sandboxId: "sandbox-test" } },
649+
{
650+
name: "delete template",
651+
command: sandboxTemplateDelete,
652+
flags: { templateId: "template-test" },
653+
},
654+
])("$name releases complete and stalled response bodies", async ({ command, flags }) => {
655+
vi.spyOn(process.stdout, "write").mockImplementation(() => true);
656+
const client = new Client({
657+
identity: {
658+
binName: "bl",
659+
version: "test",
660+
npmPackage: "bailian-cli",
661+
clientName: "bailian-cli",
662+
},
663+
settings: SETTINGS,
664+
baseUrl: "https://dashscope.aliyuncs.com",
665+
apiCred: {
666+
token: "local-test-only",
667+
baseUrl: "https://dashscope.aliyuncs.com",
668+
source: "flag",
669+
},
670+
});
671+
for (const complete of [true, false]) {
672+
const body = new ReadableStream<Uint8Array>({
673+
start(controller) {
674+
controller.enqueue(new TextEncoder().encode("unused response"));
675+
if (complete) controller.close();
676+
},
677+
});
678+
const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(body));
679+
await command.run({
680+
identity: { binName: "bl" },
681+
settings: SETTINGS,
682+
flags: { workspaceId: "ws-test", ...flags },
683+
client,
684+
} as never);
685+
const signal = fetchMock.mock.calls[0][1]!.signal!;
686+
expect(signal.aborted).toBe(true);
687+
expect(getEventListeners(signal, "abort")).toHaveLength(0);
688+
expect(body.locked).toBe(false);
689+
fetchMock.mockRestore();
690+
}
691+
});
692+
});

‎packages/core/src/client/http.ts‎

Lines changed: 132 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ import { maskToken } from "../utils/token.ts";
77
import { sourceConfig, trackingHeaders } from "./headers.ts";
88
import { Dispatcher, getGlobalDispatcher } from "undici";
99

10-
// 请求已由 --timeout 对应的 signal 控制超时。这里在实际发送前关闭底层默认的
11-
// 300 秒响应头超时,避免用户设置更长等待时间后,请求仍在 300 秒时被提前中断。
12-
// 只覆盖 headersTimeout,响应体的空闲超时保持原值。
13-
const withoutHeadersTimeout: Dispatcher.DispatcherComposeInterceptor =
10+
// Core owns the request deadline and body idle window. Override transport
11+
// limits only for this request, preserving the current proxy/connection pool.
12+
const withoutTransportTimeouts: Dispatcher.DispatcherComposeInterceptor =
1413
(dispatch) => (options, handler) =>
15-
dispatch({ ...options, headersTimeout: 0 }, handler);
14+
dispatch({ ...options, headersTimeout: 0, bodyTimeout: 0 }, handler);
1615

1716
/** 传输层依赖:UA 用 identity,timeout/verbose 用 settings。凭证由调用方(Client)注头。 */
1817
export interface HttpDeps {
@@ -42,14 +41,19 @@ function bodyReferencesOssUrl(body: unknown): boolean {
4241
return JSON.stringify(body).includes("oss://");
4342
}
4443

44+
/** Successful raw response bodies must be consumed or explicitly cancelled by the caller. */
4545
export async function request(deps: HttpDeps, opts: RequestOpts): Promise<Response> {
4646
const timeoutMs = (opts.timeout ?? deps.settings.timeout) * 1000;
4747
const requestSignal = createRequestSignal(timeoutMs, opts.signal);
4848
try {
49-
// Raw/streaming callers retain ownership of successful response bodies.
50-
return await performRequest(deps, opts, requestSignal.signal);
51-
} finally {
49+
const response = await performRequest(deps, opts, requestSignal.signal);
50+
// After headers, raw/streaming responses use a rolling idle window rather
51+
// than a total deadline. The body owns cleanup until EOF/cancel/error.
52+
return protectResponseBody(response, requestSignal);
53+
} catch (error) {
5254
requestSignal.cleanup();
55+
if (requestSignal.signal.aborted) throw requestSignal.signal.reason;
56+
throw error;
5357
}
5458
}
5559

@@ -94,12 +98,16 @@ async function performRequest(
9498
: JSON.stringify(opts.body)
9599
: undefined,
96100
signal,
97-
// 复用当前客户端,仅关闭本次请求的响应头超时。
98-
// 从原型调用 compose,兼容没有 compose 方法的客户端;类型断言适配 Node 的 fetch 类型。
99-
dispatcher: Dispatcher.prototype.compose.call(
100-
getGlobalDispatcher(),
101-
withoutHeadersTimeout,
102-
) as unknown as RequestInit["dispatcher"],
101+
// Bun's native fetch ignores dispatchers and its undici shim has no compose.
102+
// Keep native fetch in both runtimes, adapting only transport controls.
103+
...(process.versions.bun
104+
? { timeout: false as const }
105+
: {
106+
dispatcher: Dispatcher.prototype.compose.call(
107+
getGlobalDispatcher(),
108+
withoutTransportTimeouts,
109+
) as unknown as RequestInit["dispatcher"],
110+
}),
103111
};
104112
const res = await fetch(opts.url, requestInit);
105113

@@ -125,24 +133,127 @@ async function performRequest(
125133
return res;
126134
}
127135

128-
function createRequestSignal(
129-
timeoutMs: number,
130-
parentSignal?: AbortSignal,
131-
): { signal: AbortSignal; cleanup: () => void } {
136+
interface RequestSignal {
137+
signal: AbortSignal;
138+
cleanup: () => void;
139+
abort: (reason?: unknown) => void;
140+
resetIdleTimeout: () => void;
141+
pauseTimeout: () => void;
142+
}
143+
144+
function createRequestSignal(timeoutMs: number, parentSignal?: AbortSignal): RequestSignal {
132145
const controller = new AbortController();
133-
const timeout = setTimeout(() => controller.abort(), timeoutMs);
146+
let timeout: ReturnType<typeof setTimeout> | undefined;
147+
let finished = false;
148+
const armTimer = (reason?: unknown) => {
149+
if (finished) return;
150+
clearTimeout(timeout);
151+
timeout = setTimeout(() => controller.abort(reason), timeoutMs);
152+
};
134153
const abortFromParent = () => controller.abort(parentSignal?.reason);
135154
const cleanup = () => {
155+
finished = true;
136156
clearTimeout(timeout);
137157
parentSignal?.removeEventListener("abort", abortFromParent);
138158
controller.signal.removeEventListener("abort", cleanup);
139159
};
140160

141161
controller.signal.addEventListener("abort", cleanup, { once: true });
142162
if (parentSignal?.aborted) abortFromParent();
143-
else parentSignal?.addEventListener("abort", abortFromParent, { once: true });
163+
else {
164+
parentSignal?.addEventListener("abort", abortFromParent, { once: true });
165+
armTimer();
166+
}
167+
168+
return {
169+
signal: controller.signal,
170+
cleanup,
171+
abort: (reason) => controller.abort(reason),
172+
pauseTimeout: () => clearTimeout(timeout),
173+
resetIdleTimeout: () =>
174+
armTimer(
175+
new BailianError(
176+
`Response body idle timeout: no data received for ${timeoutMs / 1000}s.`,
177+
ExitCode.TIMEOUT,
178+
"Increase --timeout to allow a longer interval between response chunks.",
179+
),
180+
),
181+
};
182+
}
144183

145-
return { signal: controller.signal, cleanup };
184+
function protectResponseBody(response: Response, requestSignal: RequestSignal): Response {
185+
if (!response.body) {
186+
requestSignal.cleanup();
187+
return response;
188+
}
189+
// Only count time spent waiting for network data, not consumer processing.
190+
// An unused raw response must not keep a command alive through a timer.
191+
requestSignal.pauseTimeout();
192+
const reader = response.body.getReader();
193+
const { signal } = requestSignal;
194+
let finished = false;
195+
let onAbort: () => void;
196+
const finish = () => {
197+
finished = true;
198+
signal.removeEventListener("abort", onAbort);
199+
requestSignal.cleanup();
200+
};
201+
const cancelSource = (reason?: unknown) =>
202+
reader.cancel(reason).finally(() => reader.releaseLock());
203+
const body = new ReadableStream<Uint8Array>(
204+
{
205+
start(controller) {
206+
onAbort = () => {
207+
if (finished) return;
208+
finish();
209+
controller.error(signal.reason);
210+
void cancelSource(signal.reason).catch(() => {});
211+
};
212+
signal.addEventListener("abort", onAbort, { once: true });
213+
if (signal.aborted) onAbort();
214+
},
215+
async pull(controller) {
216+
try {
217+
requestSignal.resetIdleTimeout();
218+
const part = await reader.read();
219+
requestSignal.pauseTimeout();
220+
if (finished) return;
221+
if (part.done) {
222+
finish();
223+
reader.releaseLock();
224+
controller.close();
225+
} else {
226+
controller.enqueue(part.value);
227+
}
228+
} catch (error) {
229+
if (finished) return;
230+
finish();
231+
reader.releaseLock();
232+
controller.error(signal.aborted ? signal.reason : error);
233+
}
234+
},
235+
cancel(reason) {
236+
finish();
237+
// Own transport teardown as well as reader cleanup: cancelling the
238+
// reader alone can leave native Bun's HTTP connection open.
239+
requestSignal.abort(reason);
240+
return cancelSource(reason).catch(() => {});
241+
},
242+
},
243+
{ highWaterMark: 0 },
244+
);
245+
const protectedResponse = new Response(body, {
246+
status: response.status,
247+
statusText: response.statusText,
248+
headers: response.headers,
249+
});
250+
// A constructed Response otherwise loses fetch's URL/redirect metadata.
251+
Object.defineProperties(protectedResponse, {
252+
url: { value: response.url },
253+
redirected: { value: response.redirected },
254+
type: { value: response.type },
255+
});
256+
return protectedResponse;
146257
}
147258

148259
export async function requestJson<T>(deps: HttpDeps, opts: RequestOpts): Promise<T> {

‎packages/core/src/client/stream.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ export async function* parseSSE(response: Response): AsyncGenerator<ServerSentEv
130130
yield { data: event.data, event: event.event, id: event.id };
131131
}
132132
} finally {
133+
// A consumer may stop at [DONE] before network EOF. Cancel the body so
134+
// core HTTP releases its idle timer, abort listener, and connection.
135+
await reader.cancel().catch(() => {});
133136
reader.releaseLock();
134137
}
135138
}

0 commit comments

Comments
 (0)