-
Notifications
You must be signed in to change notification settings - Fork 883
fix(combos): fail over zero-output stream failures #2433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,6 +108,7 @@ ocx combo set balanced \ | |
| | 결과 | 동작 | | ||
| | --- | --- | | ||
| | HTTP 401, 403, 404, 408, 429, 또는 모든 5xx | 대상을 쿨다운으로 보내고 다음 적합한 대상으로 넘어갑니다. | | ||
| | 모델 수명 종료, retired, deprecated, sunset, decommissioned, 또는 더 이상 사용할 수 없다는 신호가 명시된 HTTP 410 | 해당 대상만 쿨다운으로 보내고 다음 대상으로 넘어갑니다. 관련 없는 410은 종결 오류로 유지합니다. | | ||
| | 인증, 구독, 쿼터, 속도 제한, 과부하, 또는 상위 서버 오류로 분류됨 | 상태 코드만으로는 충분하지 않더라도 대상을 쿨다운으로 보내고 넘어갑니다. | | ||
| | 클라이언트 취소(499), `origin_rejected`, cyber-policy refusal, context overflow, 또는 invalid request | 멈추고 오류를 반환합니다. 다른 대상을 써도 요청이 유효해지지 않기 때문입니다. | | ||
| | 그 밖의 분류되지 않은 오류 | 멈추고 오류를 반환합니다. | | ||
|
|
@@ -120,6 +121,8 @@ ocx combo set balanced \ | |
| 페일오버는 의도적으로 범위를 제한합니다. 대상별 가용성, 인증, 쿼터, 과부하 실패에는 도움이 되지만, 호출자 오류나 정책 거부를 숨기지는 않습니다. | ||
| ::: | ||
|
|
||
| 스트리밍 요청에서는 상위 HTTP 상태만으로 최종 결정을 내리지 않습니다. OpenCodex는 선택한 하위 대상의 Responses SSE를 출력 시작 전의 제한된 구간까지만 버퍼링합니다. 텍스트, 추론, 도구 호출 또는 그 밖의 출력 이벤트가 시작되기 전에 재시도 가능한 `response.failed` 종결 이벤트가 오면 해당 시도를 실패로 기록하고 다음 적합한 대상을 시도할 수 있습니다. 출력이 시작되거나 버퍼 상한에 도달하면 현재 대상에 커밋하며, 이후의 스트림 실패를 다른 공급자에서 다시 실행하지 않습니다. 따라서 텍스트와 도구 실행이 중복되지 않습니다. | ||
|
|
||
|
Comment on lines
+124
to
+125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Limit the documented retry to retryable The sentence currently says that a terminal As per path instructions: translated locale pages must stay consistent with actual CLI/API behavior. 🤖 Prompt for AI AgentsSource: Path instructions |
||
| ## 기본 reasoning effort | ||
|
|
||
| `defaultEffort`는 다음 조건이 모두 참일 때만 `reasoning.effort`를 채웁니다. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| import type { ResponsesTerminalStatus } from "../../bridge"; | ||
| import type { RequestLogContext } from "../request-log"; | ||
| import { createSseInspector } from "../relay"; | ||
| import { MAX_CLIENT_SSE_FRAME_BYTES } from "../sse-frame-buffer"; | ||
|
|
||
| const COMBO_STREAM_PREFLIGHT_MAX_BYTES = MAX_CLIENT_SSE_FRAME_BYTES; | ||
|
|
||
| const PRE_OUTPUT_CONTROL_EVENTS = new Set([ | ||
| "response.created", | ||
| "response.in_progress", | ||
| "response.queued", | ||
| "response.heartbeat", | ||
| ]); | ||
|
|
||
| const TERMINAL_EVENTS = new Set([ | ||
| "response.completed", | ||
| "response.failed", | ||
| "response.incomplete", | ||
| ]); | ||
|
|
||
| /** | ||
| * Decide when replaying the request on another combo target would risk duplicating | ||
| * client-visible output or a tool-side effect. Unknown event types commit the child | ||
| * conservatively; only the small Responses lifecycle preamble remains replayable. | ||
| */ | ||
| export function comboStreamPayloadCommitsOutput(payload: unknown): boolean { | ||
| if (!payload || typeof payload !== "object" || Array.isArray(payload)) return true; | ||
| const type = (payload as { type?: unknown }).type; | ||
| if (typeof type !== "string") return true; | ||
| return !PRE_OUTPUT_CONTROL_EVENTS.has(type) && !TERMINAL_EVENTS.has(type); | ||
| } | ||
|
|
||
| function replayBufferedResponse( | ||
| response: Response, | ||
| reader: ReadableStreamDefaultReader<Uint8Array>, | ||
| buffered: Uint8Array[], | ||
| ): Response { | ||
| let index = 0; | ||
| const body = new ReadableStream<Uint8Array>({ | ||
| async pull(controller) { | ||
| if (index < buffered.length) { | ||
| controller.enqueue(buffered[index++]!); | ||
| return; | ||
| } | ||
| try { | ||
| const next = await reader.read(); | ||
| if (next.done) controller.close(); | ||
| else controller.enqueue(next.value); | ||
| } catch (error) { | ||
| try { controller.error(error); } catch { /* consumer already closed */ } | ||
| } | ||
| }, | ||
| cancel(reason) { | ||
| reader.cancel(reason).catch(() => undefined); | ||
| }, | ||
| }); | ||
| return new Response(body, { | ||
| status: response.status, | ||
| statusText: response.statusText, | ||
| headers: response.headers, | ||
| }); | ||
| } | ||
|
|
||
| function failedTerminalResponse( | ||
| response: Response, | ||
| terminalPayload: Record<string, unknown>, | ||
| logCtx: RequestLogContext, | ||
| ): Response { | ||
| const nested = terminalPayload.response; | ||
| const terminalResponse = nested && typeof nested === "object" && !Array.isArray(nested) | ||
| ? nested as Record<string, unknown> | ||
| : {}; | ||
| const nestedError = terminalResponse.error; | ||
| const error = nestedError && typeof nestedError === "object" && !Array.isArray(nestedError) | ||
| ? nestedError as Record<string, unknown> | ||
| : { | ||
| type: "upstream_error", | ||
| code: "upstream_server_error", | ||
| message: logCtx.upstreamError ?? "Provider stream failed before producing output", | ||
| }; | ||
| const headers = new Headers(response.headers); | ||
| headers.set("content-type", "application/json"); | ||
| headers.delete("content-length"); | ||
| headers.delete("content-encoding"); | ||
| const usage = terminalResponse.usage; | ||
| return new Response(JSON.stringify({ | ||
| error, | ||
| // The combo classifier needs only the error and optional usage. Do not carry | ||
| // response ids, provider metadata, or future terminal fields into the client | ||
| // error envelope merely because they shared the terminal snapshot. | ||
| response: { | ||
| error, | ||
| ...(usage && typeof usage === "object" && !Array.isArray(usage) ? { usage } : {}), | ||
| }, | ||
| }), { | ||
| status: logCtx.terminalHttpStatus ?? 502, | ||
| headers, | ||
| }); | ||
| } | ||
|
|
||
| export type ComboStreamPreflightResult = | ||
| | { kind: "accepted"; response: Response } | ||
| | { kind: "failed"; response: Response }; | ||
|
|
||
| /** | ||
| * Buffer a combo child's downstream SSE only until the request becomes unsafe to | ||
| * replay or reaches a terminal. This owns exactly one body reader. The aggregate | ||
| * buffer is capped; hitting the cap commits the current target instead of growing | ||
| * memory or guessing that replay is safe. | ||
| */ | ||
| export async function preflightComboStreamResponse( | ||
| response: Response, | ||
| logCtx: RequestLogContext, | ||
| ): Promise<ComboStreamPreflightResult> { | ||
| const contentType = response.headers.get("content-type")?.toLowerCase() ?? ""; | ||
| if (!response.ok || !response.body || !contentType.includes("text/event-stream")) { | ||
| return { kind: "accepted", response }; | ||
| } | ||
|
|
||
| const reader = response.body.getReader(); | ||
| const buffered: Uint8Array[] = []; | ||
| let bufferedBytes = 0; | ||
| let outputCommitted = false; | ||
| let terminalStatus: ResponsesTerminalStatus | undefined; | ||
| let failedPayload: Record<string, unknown> | undefined; | ||
| const inspector = createSseInspector({ | ||
| logCtx, | ||
| onParsedPayload: payload => { | ||
| if (comboStreamPayloadCommitsOutput(payload)) outputCommitted = true; | ||
| if (!payload || typeof payload !== "object" || Array.isArray(payload)) return; | ||
| if ((payload as { type?: unknown }).type === "response.failed") { | ||
| failedPayload = payload as Record<string, unknown>; | ||
| } | ||
| }, | ||
| onTerminal: status => { terminalStatus = status; }, | ||
| }); | ||
|
|
||
| try { | ||
| for (;;) { | ||
| const next = await reader.read(); | ||
| if (next.done) { | ||
| inspector.finish(); | ||
| } else { | ||
| if (bufferedBytes + next.value.byteLength > COMBO_STREAM_PREFLIGHT_MAX_BYTES) { | ||
| // Keep the cap about memory the preflight allocates. The upstream chunk already exists; | ||
| // copying it before committing would transiently exceed the boundary for no | ||
| // replay benefit. Preserve it unsliced behind the already-bounded prefix. | ||
| return { | ||
| kind: "accepted", | ||
| response: replayBufferedResponse(response, reader, [...buffered, next.value]), | ||
| }; | ||
| } | ||
| const retained = next.value.slice(); | ||
| buffered.push(retained); | ||
| bufferedBytes += retained.byteLength; | ||
| inspector.feed(retained); | ||
| } | ||
|
|
||
| if (terminalStatus === "failed" && !outputCommitted && failedPayload) { | ||
| await reader.cancel("retrying zero-output combo stream failure").catch(() => undefined); | ||
| return { kind: "failed", response: failedTerminalResponse(response, failedPayload, logCtx) }; | ||
| } | ||
| if (next.done || terminalStatus !== undefined || outputCommitted | ||
| || bufferedBytes >= COMBO_STREAM_PREFLIGHT_MAX_BYTES) { | ||
| return { kind: "accepted", response: replayBufferedResponse(response, reader, buffered) }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } finally { | ||
| inspector.dispose(); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.