fix(providers): bound the OpenAI response body read, restore retries, and fail on reported generation failures - #6281
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryCursor Bugbot is generating a summary for commit c0e78c8. Configure here. |
Greptile SummaryThe PR bounds OpenAI Responses body reads, restores status-based retries with cancellable backoff, rejects reported generation failures, and improves transport-error diagnostics.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains, and the current implementation addresses both previously reported issues by bounding non-2xx body reads and making retry backoff respond immediately to cancellation.
|
| Filename | Overview |
|---|---|
| apps/sim/providers/openai/core.ts | Adds bounded body reads, status retries, response-status validation, cancellation-aware backoff, and phase-annotated transport errors; both previously reported issues are addressed. |
| apps/sim/executor/handlers/agent/agent-handler.ts | Walks wrapped causes to translate transport aborts and deadlines into a diagnostic provider-timeout message. |
| apps/sim/providers/openai/core.retry.test.ts | Covers retryable statuses, retry limits, server-directed pacing, caller cancellation, and non-retryable body stalls. |
| apps/sim/providers/openai/core.transport-phase.test.ts | Covers body deadlines, transport-phase metadata, caller abort behavior, and unbounded generation time-to-headers. |
| apps/sim/providers/openai/core.response-status.test.ts | Covers failed and incomplete generation handling, truncated tool calls, healthy responses, and continuation turns. |
| apps/sim/executor/handlers/agent/agent-handler.test.ts | Verifies wrapped TimeoutError and AbortError classification while preserving provider diagnostics. |
| apps/sim/providers/types.ts | Updates provider response typing to support the changed OpenAI response handling. |
Sequence Diagram
sequenceDiagram
participant Agent as Agent Handler
participant Provider as OpenAI Provider
participant API as OpenAI Responses API
Agent->>Provider: Execute request
Provider->>API: POST /v1/responses
alt Retryable HTTP status
API-->>Provider: 408/409/429/5xx
Provider->>Provider: Cancellable backoff
Provider->>API: Retry (up to 2 times)
else Successful headers
API-->>Provider: 200 response
Provider->>Provider: Arm body deadline
Provider->>Provider: Parse and validate status
Provider-->>Agent: Content/tool result
else Body stalls
Provider->>Provider: Abort body read after 60s
Provider-->>Agent: Annotated transport error
end
Reviews (2): Last reviewed commit: "fix(providers): bound stalled error bodi..." | Re-trigger Greptile
…ing phase A stalled `/v1/responses` call surfaced only the runtime's own `TimeoutError: The operation timed out.` after a variable wall under five minutes, with no way to tell "never answered" from "answered but the body never completed" — opposite causes with opposite fixes. - Bound the body read (60s) but never time-to-headers: `/v1/responses` withholds its 200 until generation finishes, so all think time is time-to-headers (measured: 14545ms to headers, 1ms of body). - Annotate opaque transport failures with the phase, status, ttfb, content-length and `x-request-id` — the last being the only handle OpenAI support can trace a call by. The annotation rides the error message, which reaches the trace span; traces persist even when a task stops shipping logs. - Carry the cause through `ProviderError` so the agent handler can still classify a transport timeout after rewrapping overwrites `name`. - Bound non-JSON error bodies so a gateway HTML page cannot become the user-facing block error. Deliberately no retry: `/v1/responses` ignores `Idempotency-Key` (verified live — same key and body returns two distinct response ids), so a retry would generate and bill a second response. The OpenAI SDK and the AI SDK both decline to retry this class.
…path `/v1/responses` posted through the OpenAI SDK until 1933e1a (#3135) moved it onto raw `fetch`, which silently dropped the SDK's `maxRetries: 2`. The 16 other providers that construct an SDK client still retry; only openai and azure-openai, which share this core, retried nothing. Restores 2 retries (3 attempts) on 408/409/429/5xx using `backoffWithJitter` and `parseRetryAfter`, preferring OpenAI's `retry-after-ms` over `Retry-After`. The loop sits in the shared request helper so the streaming paths are covered too — a refused request yields no body, so no stream bytes were consumed and no response was created server-side. Aborts, other 4xx, and the body-read deadline stay non-retryable. `/v1/responses` ignores `Idempotency-Key`, so a retry after a response already exists would generate and bill a second one; the body stall is exactly that case.
…a failed generation The non-streaming Responses path read only `output`, never `status` or `error`. `/v1/responses` answers HTTP 200 for generations that did not succeed, so a failed run reached the user as a SUCCESS with empty content and billed tokens — while `deriveOpenAIFinishReason` independently wrote `finishReason: 'error'` onto the same trace span, leaving the trace and the block contradicting each other. - Reject a 200 carrying `error != null` or `status: 'failed'`, surfacing the API's own message (and error code when present) rather than a generic string. Matches the vendored AI SDK, which throws an APICallError on a 200 carrying `error`. - Pin the `incomplete` policy to the one `streamResponsesTurn` already applies, so the two loops cannot diverge again: tolerate only `max_output_tokens` truncation with no function call and return the partial prose; error on every other reason, and on any incomplete that truncated a tool call. A truncated `function_call` holds half-written JSON, and executing it made `parseToolArguments` throw — reporting a tool bug instead of the truncation that actually happened. - Gate tool execution on a finished generation, mirroring `toolsExecutable` in the streaming loop. The assertion sits in `postResponses` so the first turn and every tool-loop continuation are covered by construction, and outside the transport `try` so a rejected generation is never misreported as a body stall. A status the API did not send is not asserted against: this path is shared with Azure OpenAI and OpenAI-compatible gateways, and inventing a failure for an absent field would break healthy responses rather than report broken ones.
c0e78c8 to
f403ceb
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f403ceb. Configure here.
|
Superseded by #6283. The root cause turned out to be different from what this PR was built on. Confirmed from OpenAI's stored responses for the actual incident: the model repeated one tool call until it consumed the full 128,000-token output budget — The response-body deadline here was built for a body-stall theory the evidence disproved, and the comments and commit messages document that wrong cause. #6283 keeps what holds up — phase annotation, The status-based retries from this branch are still worth having and will follow separately; they're unrelated to this incident. |
Summary
/v1/responsesbody read (60s) so a stalled response fails fast instead of hanging ~4.5min on the runtime's own socket wall. Time-to-headers stays unbounded — the endpoint withholds its 200 until generation finishes, so all think time is time-to-headers (measured: 14545ms to headers, 1ms of body).awaiting-response-headersvsreading-response-body) plus status, ttfb, content-length andx-request-id. The annotation rides the error message into the trace span, which persists even when a task stops shipping logs.Retry-After). This path lost them in improvement(openai): migrate to responses api #3135 when it moved off the OpenAI SDK onto rawfetch; every other provider still retries via its SDK client.status: "failed"/ populatederrorpreviously returned success with empty content and billed tokens, and a truncatedfunction_callwas executed. Now matches the streaming path.Deliberately no retry on a stalled body:
/v1/responsesignoresIdempotency-Key(verified live — same key and body returns two distinct response ids), so a retry would generate and bill a second response. The OpenAI SDK and the AI SDK both decline to retry this class.Type of Change
Testing
Tested manually against the live API plus 40 new unit tests.
/v1/responsespayloads (completed, incomplete/max_output_tokens, tool-call) through the provider — none rejected by the new status guard.Idempotency-Keyis ignored on this endpoint, and that headers arrive only when generation completes.executor/handlers/pi/cloud-review-tools.test.ts) reproduces identically onstaging.Checklist