Skip to content

fix(providers): bound the OpenAI response body read, restore retries, and fail on reported generation failures - #6281

Closed
waleedlatif1 wants to merge 5 commits into
stagingfrom
fix/openai-provider-transport-diagnostics
Closed

fix(providers): bound the OpenAI response body read, restore retries, and fail on reported generation failures#6281
waleedlatif1 wants to merge 5 commits into
stagingfrom
fix/openai-provider-transport-diagnostics

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Bound the /v1/responses body 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).
  • Annotate opaque transport failures with the phase they died in (awaiting-response-headers vs reading-response-body) plus status, ttfb, content-length and x-request-id. The annotation rides the error message into the trace span, which persists even when a task stops shipping logs.
  • Restore status-based retries (408/409/429/5xx, 2 retries, backoff + Retry-After). This path lost them in improvement(openai): migrate to responses api #3135 when it moved off the OpenAI SDK onto raw fetch; every other provider still retries via its SDK client.
  • Fail the block when a 200 reports a failed generation. status: "failed" / populated error previously returned success with empty content and billed tokens, and a truncated function_call was executed. Now matches the streaming path.

Deliberately no retry on a stalled body: /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.

Type of Change

  • Bug fix

Testing

Tested manually against the live API plus 40 new unit tests.

  • Replayed real captured /v1/responses payloads (completed, incomplete/max_output_tokens, tool-call) through the provider — none rejected by the new status guard.
  • Verified live that Idempotency-Key is ignored on this endpoint, and that headers arrive only when generation completes.
  • Verified the body deadline propagates into an already-returned body under the real runtime.
  • Every new test verified able to fail by breaking the implementation first.
  • Full suite green; one unrelated pre-existing failure (executor/handlers/pi/cloud-review-tools.test.ts) reproduces identically on staging.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 5, 2026 4:17am

Request Review

@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Cursor Bugbot is generating a summary for commit c0e78c8. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR bounds OpenAI Responses body reads, restores status-based retries with cancellable backoff, rejects reported generation failures, and improves transport-error diagnostics.

  • Adds a 60-second deadline for successful and error response-body reads.
  • Retries 408, 409, 429, and 5xx responses while honoring server pacing.
  • Validates non-streaming response status before returning content or executing tools.
  • Preserves transport phase and response metadata through provider error wrapping.
  • Adds focused tests for retries, deadlines, failure statuses, and agent-level error translation.

Confidence Score: 5/5

The 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.

Important Files Changed

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
Loading

Reviews (2): Last reviewed commit: "fix(providers): bound stalled error bodi..." | Re-trigger Greptile

Comment thread apps/sim/providers/openai/core.ts
Comment thread apps/sim/providers/openai/core.ts Outdated
Comment thread apps/sim/providers/openai/core.ts Outdated
…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.
@waleedlatif1
waleedlatif1 force-pushed the fix/openai-provider-transport-diagnostics branch from c0e78c8 to f403ceb Compare August 5, 2026 04:17
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@waleedlatif1
waleedlatif1 deleted the fix/openai-provider-transport-diagnostics branch August 5, 2026 05:29
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

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 — status: incomplete, reason: max_output_tokens, 0 reasoning tokens, 287 tokens of stored output against 128,000 billed. /v1/responses withholds its 200 until generation completes, so the client was stuck in the headers phase, never the body phase.

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, x-request-id, cause propagation, the failed-generation guard, bounded error bodies — off a clean base with the corrected narrative.

The status-based retries from this branch are still worth having and will follow separately; they're unrelated to this incident.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant