Skip to content

Commit e228026

Browse files
zhaozhuang521zhaozhuang521
authored
feat: add request timeout, retry and AbortSignal support (agentscope-ai#5764)
1 parent 66051e6 commit e228026

2 files changed

Lines changed: 329 additions & 28 deletions

File tree

‎console/src/api/request.test.ts‎

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,221 @@ describe("request", () => {
154154
expect.any(Object),
155155
);
156156
});
157+
158+
// ---------------------------------------------------------------------------
159+
// Timeout handling
160+
// ---------------------------------------------------------------------------
161+
162+
it("aborts request after default timeout (30s)", async () => {
163+
vi.useFakeTimers();
164+
const abortSpy = vi.spyOn(AbortController.prototype, "abort");
165+
166+
global.fetch = vi.fn().mockImplementation((_url, options) => {
167+
return new Promise((_resolve, reject) => {
168+
// Listen for abort signal
169+
if (options?.signal) {
170+
options.signal.addEventListener("abort", () => {
171+
reject(new DOMException("The operation was aborted", "AbortError"));
172+
});
173+
}
174+
});
175+
});
176+
177+
const requestPromise = request("/slow-endpoint");
178+
vi.advanceTimersByTime(30000);
179+
180+
await expect(requestPromise).rejects.toThrow("Request timeout");
181+
expect(abortSpy).toHaveBeenCalled();
182+
183+
abortSpy.mockRestore();
184+
vi.useRealTimers();
185+
});
186+
187+
it("respects custom timeout option", async () => {
188+
vi.useFakeTimers();
189+
const abortSpy = vi.spyOn(AbortController.prototype, "abort");
190+
191+
global.fetch = vi.fn().mockImplementation((_url, options) => {
192+
return new Promise((_resolve, reject) => {
193+
if (options?.signal) {
194+
options.signal.addEventListener("abort", () => {
195+
reject(new DOMException("The operation was aborted", "AbortError"));
196+
});
197+
}
198+
});
199+
});
200+
201+
const requestPromise = request("/slow-endpoint", { timeout: 5000 });
202+
vi.advanceTimersByTime(5000);
203+
204+
await expect(requestPromise).rejects.toThrow("Request timeout");
205+
expect(abortSpy).toHaveBeenCalled();
206+
207+
abortSpy.mockRestore();
208+
vi.useRealTimers();
209+
});
210+
211+
it("completes successfully before timeout", async () => {
212+
vi.useFakeTimers();
213+
mockFetch(200, { data: "ok" });
214+
215+
const requestPromise = request("/fast-endpoint", { timeout: 10000 });
216+
vi.advanceTimersByTime(100);
217+
218+
const result = await requestPromise;
219+
expect(result).toEqual({ data: "ok" });
220+
221+
vi.useRealTimers();
222+
});
223+
224+
it("passes AbortSignal to fetch", async () => {
225+
mockFetch(200, { data: "ok" });
226+
await request("/models");
227+
228+
const fetchOptions = (fetch as any).mock.calls[0][1];
229+
expect(fetchOptions.signal).toBeDefined();
230+
expect(fetchOptions.signal).toBeInstanceOf(AbortSignal);
231+
});
232+
233+
it("retries on timeout when retries option is set", async () => {
234+
vi.useFakeTimers();
235+
let attemptCount = 0;
236+
237+
global.fetch = vi.fn().mockImplementation((_url, options) => {
238+
attemptCount++;
239+
return new Promise((_resolve, reject) => {
240+
if (options?.signal) {
241+
options.signal.addEventListener("abort", () => {
242+
reject(new DOMException("The operation was aborted", "AbortError"));
243+
});
244+
}
245+
});
246+
});
247+
248+
const requestPromise = request("/flaky-endpoint", {
249+
timeout: 1000,
250+
retries: 2,
251+
retryDelay: 100,
252+
});
253+
254+
// First attempt timeout
255+
vi.advanceTimersByTime(1000);
256+
await vi.advanceTimersByTimeAsync(100); // retry delay
257+
258+
// Second attempt timeout
259+
vi.advanceTimersByTime(1000);
260+
await vi.advanceTimersByTimeAsync(100); // retry delay
261+
262+
// Third attempt timeout
263+
vi.advanceTimersByTime(1000);
264+
265+
await expect(requestPromise).rejects.toThrow("Request timeout");
266+
expect(attemptCount).toBe(3); // initial + 2 retries
267+
268+
vi.useRealTimers();
269+
});
270+
271+
it("succeeds on retry after initial timeout", async () => {
272+
vi.useFakeTimers();
273+
let attemptCount = 0;
274+
275+
global.fetch = vi.fn().mockImplementation((_url, options) => {
276+
attemptCount++;
277+
return new Promise((resolve, reject) => {
278+
if (attemptCount === 1) {
279+
// First attempt: timeout
280+
if (options?.signal) {
281+
options.signal.addEventListener("abort", () => {
282+
reject(
283+
new DOMException("The operation was aborted", "AbortError"),
284+
);
285+
});
286+
}
287+
} else {
288+
// Second attempt: success
289+
resolve({
290+
ok: true,
291+
status: 200,
292+
statusText: "OK",
293+
headers: { get: () => "application/json" },
294+
json: () => Promise.resolve({ data: "success" }),
295+
text: () => Promise.resolve(JSON.stringify({ data: "success" })),
296+
} as unknown as Response);
297+
}
298+
});
299+
});
300+
301+
const requestPromise = request("/flaky-endpoint", {
302+
timeout: 1000,
303+
retries: 1,
304+
retryDelay: 100,
305+
});
306+
307+
// First attempt timeout
308+
vi.advanceTimersByTime(1000);
309+
await vi.advanceTimersByTimeAsync(100); // retry delay
310+
311+
const result = await requestPromise;
312+
expect(result).toEqual({ data: "success" });
313+
expect(attemptCount).toBe(2);
314+
315+
vi.useRealTimers();
316+
});
317+
318+
// ---------------------------------------------------------------------------
319+
// External AbortSignal handling
320+
// ---------------------------------------------------------------------------
321+
322+
it("does not send request when caller signal is already aborted", async () => {
323+
mockFetch(200, { data: "ok" });
324+
325+
const controller = new AbortController();
326+
controller.abort();
327+
328+
await expect(
329+
request("/models", { signal: controller.signal }),
330+
).rejects.toThrow("aborted");
331+
332+
expect(fetch).not.toHaveBeenCalled();
333+
});
334+
335+
it("does not retry when caller manually aborts (not timeout)", async () => {
336+
let attemptCount = 0;
337+
338+
global.fetch = vi.fn().mockImplementation((_url, options) => {
339+
attemptCount++;
340+
return new Promise((_resolve, reject) => {
341+
if (options?.signal) {
342+
options.signal.addEventListener("abort", () => {
343+
reject(new DOMException("The operation was aborted", "AbortError"));
344+
});
345+
}
346+
});
347+
});
348+
349+
const callerController = new AbortController();
350+
const requestPromise = request("/cancel-endpoint", {
351+
timeout: 30000,
352+
retries: 3,
353+
signal: callerController.signal,
354+
});
355+
356+
// Simulate caller aborting immediately (not a timeout)
357+
callerController.abort();
358+
359+
await expect(requestPromise).rejects.toThrow("aborted");
360+
expect(attemptCount).toBe(1); // no retries for external abort
361+
});
362+
363+
it("cleans up abort listener on caller signal after successful request", async () => {
364+
mockFetch(200, { data: "ok" });
365+
366+
const callerController = new AbortController();
367+
const removeSpy = vi.spyOn(callerController.signal, "removeEventListener");
368+
369+
await request("/models", { signal: callerController.signal });
370+
371+
expect(removeSpy).toHaveBeenCalledWith("abort", expect.any(Function));
372+
removeSpy.mockRestore();
373+
});
157374
});

‎console/src/api/request.ts‎

Lines changed: 112 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -57,48 +57,132 @@ function buildHeaders(method?: string, extra?: HeadersInit): Headers {
5757
return headers;
5858
}
5959

60+
export interface RequestOptions extends RequestInit {
61+
/** Request timeout in milliseconds. Defaults to 30000 (30 seconds). */
62+
timeout?: number;
63+
/** Number of retry attempts on timeout. Defaults to 0 (no retry). */
64+
retries?: number;
65+
/** Delay between retries in milliseconds. Defaults to 1000 (1 second). */
66+
retryDelay?: number;
67+
}
68+
69+
const DEFAULT_TIMEOUT_MS = 30000;
70+
const DEFAULT_RETRIES = 0;
71+
const DEFAULT_RETRY_DELAY_MS = 1000;
72+
6073
export async function request<T = unknown>(
6174
path: string,
62-
options: RequestInit = {},
75+
options: RequestOptions = {},
6376
): Promise<T> {
6477
const url = getApiUrl(path);
6578
const method = options.method || "GET";
6679
const headers = buildHeaders(method, options.headers);
80+
const {
81+
timeout = DEFAULT_TIMEOUT_MS,
82+
retries = DEFAULT_RETRIES,
83+
retryDelay = DEFAULT_RETRY_DELAY_MS,
84+
signal: callerSignal,
85+
...fetchOptions
86+
} = options;
87+
88+
// Early exit: caller's signal already aborted before we even start
89+
if (callerSignal?.aborted) {
90+
throw new DOMException("The operation was aborted", "AbortError");
91+
}
6792

68-
const response = await fetch(url, {
69-
...options,
70-
headers,
71-
});
72-
73-
if (!response.ok) {
74-
if (response.status === 401) {
75-
clearAuthToken();
76-
if (window.location.pathname !== "/login") {
77-
window.location.href = "/login";
93+
let lastError: Error | null = null;
94+
95+
for (let attempt = 0; attempt <= retries; attempt++) {
96+
// Create AbortController for timeout handling
97+
const controller = new AbortController();
98+
let timedOut = false;
99+
const timeoutId = setTimeout(() => {
100+
timedOut = true;
101+
controller.abort();
102+
}, timeout);
103+
104+
// Wire caller's signal to our controller so external abort also works
105+
let onCallerAbort: (() => void) | undefined;
106+
if (callerSignal) {
107+
if (callerSignal.aborted) {
108+
controller.abort();
109+
} else {
110+
onCallerAbort = () => controller.abort();
111+
callerSignal.addEventListener("abort", onCallerAbort, { once: true });
78112
}
79-
throw new Error("Not authenticated");
80113
}
81114

82-
const text = await response.text().catch(() => "");
83-
const contentType = response.headers.get("content-type") || "";
84-
const errorMessage = getErrorMessageFromBody(text, contentType);
115+
try {
116+
const response = await fetch(url, {
117+
...fetchOptions,
118+
headers,
119+
signal: controller.signal,
120+
});
121+
122+
if (!response.ok) {
123+
if (response.status === 401) {
124+
clearAuthToken();
125+
if (window.location.pathname !== "/login") {
126+
window.location.href = "/login";
127+
}
128+
throw new Error("Not authenticated");
129+
}
130+
131+
const text = await response.text().catch(() => "");
132+
const contentType = response.headers.get("content-type") || "";
133+
const errorMessage = getErrorMessageFromBody(text, contentType);
134+
135+
// Preserve raw body for parseErrorDetail() to extract structured fields
136+
const finalMessage = errorMessage
137+
? `${errorMessage} - ${text}`
138+
: `Request failed: ${response.status} ${response.statusText}`;
139+
140+
throw new Error(finalMessage);
141+
}
85142

86-
// Preserve raw body for parseErrorDetail() to extract structured fields
87-
const finalMessage = errorMessage
88-
? `${errorMessage} - ${text}`
89-
: `Request failed: ${response.status} ${response.statusText}`;
143+
if (response.status === 204) {
144+
return undefined as T;
145+
}
90146

91-
throw new Error(finalMessage);
92-
}
147+
const contentType = response.headers.get("content-type") || "";
148+
if (!contentType.includes("application/json")) {
149+
return (await response.text()) as unknown as T;
150+
}
93151

94-
if (response.status === 204) {
95-
return undefined as T;
96-
}
152+
return (await response.json()) as T;
153+
} catch (error) {
154+
if (
155+
error instanceof DOMException &&
156+
error.name === "AbortError" &&
157+
!timedOut
158+
) {
159+
// External abort (caller cancelled): do not retry, rethrow as-is
160+
throw error;
161+
}
97162

98-
const contentType = response.headers.get("content-type") || "";
99-
if (!contentType.includes("application/json")) {
100-
return (await response.text()) as unknown as T;
163+
if (error instanceof DOMException && error.name === "AbortError") {
164+
// Timeout-triggered abort
165+
lastError = new Error(
166+
`Request timeout after ${timeout}ms: ${method} ${path}`,
167+
);
168+
169+
// Retry if we have attempts remaining
170+
if (attempt < retries) {
171+
await new Promise((resolve) => setTimeout(resolve, retryDelay));
172+
continue;
173+
}
174+
} else {
175+
// Non-timeout errors should not retry
176+
throw error;
177+
}
178+
} finally {
179+
clearTimeout(timeoutId);
180+
if (onCallerAbort && callerSignal) {
181+
callerSignal.removeEventListener("abort", onCallerAbort);
182+
}
183+
}
101184
}
102185

103-
return (await response.json()) as T;
186+
// All retries exhausted
187+
throw lastError;
104188
}

0 commit comments

Comments
 (0)