Skip to content

fix(proxy): return the upstream error when a collapsed Responses stream fails - #148

Merged
GoDiao merged 4 commits into
GoDiao:mainfrom
pcbeingused333:fix/responses-nonstream-failed-event
Oct 24, 2026
Merged

GoDiao merged 4 commits into
GoDiao:mainfrom
pcbeingused333:fix/responses-nonstream-failed-event

Conversation

@pcbeingused333

Copy link
Copy Markdown
Contributor

When the ChatGPT sign-in path gets a non-streaming request, it forces a streamed Responses call and collapses the stream back into one message (openaiResponsesStreamToAnthropicResponse). That function only read response.completed and response.incomplete. A stream that ends with response.failed or error fell through to the text-only fallback, so the caller got an empty end_turn message with HTTP 200 and never saw the upstream error. The streaming translator already turns both events into an Anthropic error event.

The collapse now remembers the message of a response.failed or error event (same lookup as the streaming translator, with the same fallback text) and throws OpenAIResponsesStreamError when no final response arrived. A response.completed or response.incomplete that comes later still wins. buildOpenAICodexFetch catches that error and answers with { type: 'error', error: { type: 'api_error', message } } and status 502, the shape its other upstream failures use.

This is the same collapse helper as #143, on the ChatGPT sign-in path that #119 is reworking; the change stays inside that helper and its one caller, so it carries over or drops out with that rework.

Tests: proxy-streaming.test.ts checks that response.failed and error events (with and without a message) reject with the upstream message, and that a completed response after an earlier error is kept. fetch.test.ts checks the 502 body for a failed stream on a non-streaming call. On the previous code the fetch test gets HTTP 200 and the streaming test file does not compile its import. proxy-streaming, fetch and proxy-transform tests pass (75). This was only exercised with synthetic event streams, not against a live ChatGPT/Codex upstream. The CHANGELOG line is under Unreleased.

This change was made with Claude Code.

🤖 Generated with Claude Code

pcbeingused333 and others added 2 commits October 20, 2026 15:30
…am fails

When the ChatGPT sign-in path forces a streamed Responses request for a
non-streaming call, the stream is collapsed back into one message. Only
response.completed and response.incomplete were read; response.failed
and error events were dropped, so a failed generation came back as an
empty end_turn reply with HTTP 200. The streaming translator already
turns both events into an error event.

The collapse now throws OpenAIResponsesStreamError for them when no
final response arrived, and the fetch wrapper answers with an
api_error body and status 502 like its other upstream failures.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@GoDiao GoDiao left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks again! The fix is right. fetch.ts is the only caller, so the new throw stays contained, the 502 api_error body matches the other upstream failures there, and a later response.completed still wins. The tests cover each case.

A few small things before we merge:

  1. Share the error lookup instead of copying it. upstreamErrorMessage is a copy of the lookup inside the response.failed / error case in openaiResponsesStreamToAnthropic.ts. If one copy changes later, the streaming and non-streaming paths will report different messages. Please extract it into an exported function there and call it from both places:

    // openaiResponsesStreamToAnthropic.ts
    export function upstreamErrorMessage(data: Record<string, unknown>): string {
      const response = data.response as Record<string, unknown> | undefined
      const error = (response?.error ?? data.error ?? data) as Record<string, unknown> | undefined
      return typeof error?.message === 'string' && error.message
        ? error.message
        : 'Upstream Responses stream failed'
    }

    Then const message = upstreamErrorMessage(data) in that case, and import it in openaiResponsesStreamToAnthropicResponse.ts. This touches one more file than you intended, but it's a small extraction and moves with the #119 rework just as easily.

  2. No comments in new code. The project keeps new code free of comments, including JSDoc. This isn't written down in CONTRIBUTING yet, which is on us. Please drop the JSDoc on OpenAIResponsesStreamError and the "Same message lookup…" comment; the shared function makes the second one unnecessary anyway.

  3. Rebase on main. CHANGELOG.md conflicts, because [Unreleased] on main already has a ### Fixed section. Moving your entry into it is enough.

Thanks for keeping these proxy fixes coming!

pcbeingused333 and others added 2 commits October 23, 2026 23:15
@pcbeingused333

Copy link
Copy Markdown
Contributor Author

Thanks for the review, and sorry for the delay. All three points are addressed in the pushed commits:

  1. upstreamErrorMessage is now one exported function in openaiResponsesStreamToAnthropic.ts, used by the response.failed / error case there and by openaiResponsesStreamToAnthropicResponse.ts; the copy is gone.
  2. The JSDoc on OpenAIResponsesStreamError and the "Same message lookup…" comment are removed. Neither the new code nor the new tests carry comments.
  3. Merged main into the branch (no force-push). The CHANGELOG.md entry now sits under the existing ### Fixed in [Unreleased].

proxy-streaming, fetch and proxy-transform tests still pass (75).

This update was prepared with Claude Code.

@GoDiao GoDiao left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks! All three points are in, and the tests cover each case.

@GoDiao
GoDiao merged commit 63ebed5 into GoDiao:main Oct 24, 2026
4 checks passed
@GoDiao

GoDiao commented Oct 24, 2026

Copy link
Copy Markdown
Owner

Merged, thanks @pcbeingused333! Sharing the error lookup keeps the streaming and non-streaming paths reporting the same message, and the late-completion test pins the one case that's easy to get wrong. That's your third proxy fix here, and each one has come with tests that fail on the old code, which makes them easy to review.

Your CHANGELOG entry is in. If DreamCoder is useful to you, a ⭐ on the repo would mean a lot. Looking forward to the next one!

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.

2 participants