Skip to content

Commit 94065b9

Browse files
steipeteYigtwxx
steipete
and
Yigtwxx
authored
refactor(ai): separate HTTP and transport retries
Co-authored-by: Yigtwxx <[email protected]>
1 parent 85ec538 commit 94065b9

2 files changed

Lines changed: 70 additions & 58 deletions

File tree

‎packages/ai/src/providers/openai-chatgpt-responses.retry.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,22 @@ describe("streamOpenAICodexResponses retry classification", () => {
9797
expect(result.stopReason).toBe("error");
9898
expect(fetchMock).toHaveBeenCalledTimes(2);
9999
});
100+
101+
it("does not retry a bodyless 304 response", async () => {
102+
const fetchMock = vi
103+
.fn<typeof fetch>()
104+
.mockResolvedValue(new Response(null, { status: 304, statusText: "Not Modified" }));
105+
vi.stubGlobal("fetch", fetchMock);
106+
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
107+
108+
const result = await streamOpenAICodexResponses(model, context, {
109+
apiKey,
110+
transport: "sse",
111+
}).result();
112+
113+
expect(result.stopReason).toBe("error");
114+
expect(result.errorMessage).toBe("Not Modified");
115+
expect(fetchMock).toHaveBeenCalledTimes(1);
116+
expect(setTimeoutSpy).not.toHaveBeenCalled();
117+
});
100118
});

‎packages/ai/src/providers/openai-chatgpt-responses.ts‎

Lines changed: 52 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,6 @@ interface RequestBody {
135135
// Retry Helpers
136136
// ============================================================================
137137

138-
/**
139-
* Marks a provider response the retry loop already classified as non-retryable, so the
140-
* surrounding catch (which treats unknown failures as retryable network errors) rethrows
141-
* it untouched instead of resending the request.
142-
*/
143-
class NonRetryableCodexResponseError extends Error {}
144-
145138
function isRetryableError(status: number, errorText: string): boolean {
146139
if (status === 429 || status === 500 || status === 502 || status === 503 || status === 504) {
147140
return true;
@@ -151,6 +144,32 @@ function isRetryableError(status: number, errorText: string): boolean {
151144
);
152145
}
153146

147+
function resolveHttpRetryDelayMs(response: Response, attempt: number): number {
148+
const fallbackMs = BASE_DELAY_MS * 2 ** attempt;
149+
const retryAfterMs = response.headers.get("retry-after-ms");
150+
if (retryAfterMs !== null) {
151+
const trimmed = retryAfterMs.trim();
152+
const millis = Number(trimmed);
153+
return /^\d+(?:\.\d+)?$/.test(trimmed) && Number.isFinite(millis)
154+
? (clampTimerTimeoutMs(millis, 0) ?? fallbackMs)
155+
: fallbackMs;
156+
}
157+
158+
const retryAfter = response.headers.get("retry-after");
159+
if (!retryAfter) {
160+
return fallbackMs;
161+
}
162+
const trimmed = retryAfter.trim();
163+
const seconds = Number(trimmed);
164+
if (/^\d+$/.test(trimmed) && Number.isFinite(seconds)) {
165+
return clampTimerTimeoutMs(seconds * 1000, 0) ?? fallbackMs;
166+
}
167+
const retryAt = parseRetryAfterHttpDateMs(trimmed);
168+
return retryAt === undefined
169+
? fallbackMs
170+
: (clampTimerTimeoutMs(retryAt - Date.now(), 0) ?? fallbackMs);
171+
}
172+
154173
function resolveRequestTimeoutMs(options?: OpenAICodexResponsesOptions): number | undefined {
155174
const timeoutMs = options?.timeoutMs;
156175
return typeof timeoutMs === "number" && Number.isFinite(timeoutMs) && timeoutMs > 0
@@ -380,64 +399,26 @@ export const streamOpenAICodexResponses: StreamFunction<
380399
throw new Error("Request was aborted");
381400
}
382401

402+
let attemptResponse: Response;
403+
let errorText: string;
383404
try {
384-
response = await fetch(resolveCodexUrl(model.baseUrl), {
405+
attemptResponse = await fetch(resolveCodexUrl(model.baseUrl), {
385406
method: "POST",
386407
headers: sseHeaders,
387408
body: sseBody,
388409
signal: activeSignal,
389410
});
411+
response = attemptResponse;
390412
await options?.onResponse?.(
391-
{ status: response.status, headers: headersToRecord(response.headers) },
413+
{ status: attemptResponse.status, headers: headersToRecord(attemptResponse.headers) },
392414
model,
393415
);
394416

395-
if (response.ok) {
417+
if (attemptResponse.ok) {
396418
break;
397419
}
398-
399-
const errorText = await readChatGptResponsesErrorTextLimited(response);
400-
if (attempt < maxRetries && isRetryableError(response.status, errorText)) {
401-
let delayMs = BASE_DELAY_MS * 2 ** attempt;
402-
403-
const retryAfterMs = response.headers.get("retry-after-ms");
404-
if (retryAfterMs !== null) {
405-
const trimmedRetryAfterMs = retryAfterMs.trim();
406-
const millis = Number(trimmedRetryAfterMs);
407-
if (/^\d+(?:\.\d+)?$/.test(trimmedRetryAfterMs) && Number.isFinite(millis)) {
408-
delayMs = clampTimerTimeoutMs(millis, 0) ?? delayMs;
409-
}
410-
} else {
411-
const retryAfter = response.headers.get("retry-after");
412-
if (retryAfter) {
413-
const trimmedRetryAfter = retryAfter.trim();
414-
const seconds = Number(trimmedRetryAfter);
415-
if (/^\d+$/.test(trimmedRetryAfter) && Number.isFinite(seconds)) {
416-
delayMs = clampTimerTimeoutMs(seconds * 1000, 0) ?? delayMs;
417-
} else {
418-
const retryAt = parseRetryAfterHttpDateMs(trimmedRetryAfter);
419-
if (retryAt !== undefined) {
420-
delayMs = clampTimerTimeoutMs(retryAt - Date.now(), 0) ?? delayMs;
421-
}
422-
}
423-
}
424-
}
425-
426-
await sleepWithAbort(delayMs, activeSignal);
427-
continue;
428-
}
429-
430-
// Parse error for friendly message on final attempt or non-retryable error
431-
const fakeResponse = new Response(errorText, {
432-
status: response.status,
433-
statusText: response.statusText,
434-
});
435-
const info = await parseErrorResponse(fakeResponse);
436-
throw new NonRetryableCodexResponseError(info.friendlyMessage || info.message);
420+
errorText = await readChatGptResponsesErrorTextLimited(attemptResponse);
437421
} catch (error) {
438-
if (error instanceof NonRetryableCodexResponseError) {
439-
throw error;
440-
}
441422
if (error instanceof Error) {
442423
if (
443424
isRequestTimeoutError(
@@ -466,6 +447,18 @@ export const streamOpenAICodexResponses: StreamFunction<
466447
}
467448
throw lastError;
468449
}
450+
451+
if (attempt < maxRetries && isRetryableError(attemptResponse.status, errorText)) {
452+
await sleepWithAbort(resolveHttpRetryDelayMs(attemptResponse, attempt), activeSignal);
453+
continue;
454+
}
455+
456+
const info = parseErrorResponseText(
457+
errorText,
458+
attemptResponse.status,
459+
attemptResponse.statusText,
460+
);
461+
throw new Error(info.friendlyMessage || info.message);
469462
}
470463

471464
if (!response?.ok) {
@@ -1589,11 +1582,12 @@ async function readChatGptResponsesErrorTextLimited(response: Response): Promise
15891582
return text;
15901583
}
15911584

1592-
async function parseErrorResponse(
1593-
response: Response,
1594-
): Promise<{ message: string; friendlyMessage?: string }> {
1595-
const raw = await readChatGptResponsesErrorTextLimited(response);
1596-
let message = raw || response.statusText || "Request failed";
1585+
function parseErrorResponseText(
1586+
raw: string,
1587+
status: number,
1588+
statusText: string,
1589+
): { message: string; friendlyMessage?: string } {
1590+
let message = raw || statusText || "Request failed";
15971591
let friendlyMessage: string | undefined;
15981592

15991593
try {
@@ -1611,7 +1605,7 @@ async function parseErrorResponse(
16111605
const code = err.code || err.type || "";
16121606
if (
16131607
/usage_limit_reached|usage_not_included|rate_limit_exceeded/i.test(code) ||
1614-
response.status === 429
1608+
status === 429
16151609
) {
16161610
const plan = err.plan_type ? ` (${err.plan_type.toLowerCase()} plan)` : "";
16171611
const mins = err.resets_at

0 commit comments

Comments
 (0)