Surface the error from a streamed Responses response.failed event#546
Open
PratikDhanave wants to merge 1 commit into
Open
Surface the error from a streamed Responses response.failed event#546PratikDhanave wants to merge 1 commit into
PratikDhanave wants to merge 1 commit into
Conversation
A response.failed streaming event carries its error on the response object (Response.Error), but responsesProcessStreamingUpdate had no case for ResponseFailedEvent, so it fell through to the default branch and emitted an empty update — silently dropping the failure and its message. Handle ResponseFailedEvent by emitting an ErrorContent built from Response.Error (message + code), mirroring the existing ResponseErrorEvent handling. Adds a streaming test asserting the error is surfaced; the existing failed-update test (which only checks ResponseID) still passes.
There was a problem hiding this comment.
Pull request overview
This PR fixes the OpenAI Responses streaming pathway so that response.failed events surface the embedded response error (message/code) instead of producing an empty update, making streamed failures observable to consumers.
Changes:
- Add handling for
responses.ResponseFailedEventto emitErrorContentfromResponse.Error. - Add a regression test ensuring streamed
response.failedyieldsErrorContentwith the expected message/code. - Minor formatting adjustment in
NewResponsesAgentcall site (gofmt-style).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| provider/openaiprovider/responses.go | Adds response.failed event handling to propagate response error details into streamed updates. |
| provider/openaiprovider/responses_test.go | Adds a streaming test verifying failed responses produce ErrorContent instead of an empty update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1053
to
+1058
| if event.Response.Error.Message != "" { | ||
| u.Contents = []message.Content{&message.ErrorContent{ | ||
| Message: event.Response.Error.Message, | ||
| ErrorCode: string(event.Response.Error.Code), | ||
| }} | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In the OpenAI Responses streaming path,
responsesProcessStreamingUpdatehas cases forResponseCompletedEvent,ResponseIncompleteEvent, and a top-levelResponseErrorEvent(which emitsErrorContent) — but no case forResponseFailedEvent(response.failed). A failed response carries its error on the response object (Response.Error), so it falls through to thedefaultbranch, which emits an empty update:default: u = createUpdate(message.RoleAssistant, nil) ...The result: when a streamed response fails, the framework yields updates with a
ResponseIDbut no error content and no error — the failure and its message (e.g."Internal error") are silently dropped, so a failed response is indistinguishable from an empty successful one.Fix
Add a
case responses.ResponseFailedEventthat surfaces the error asErrorContentbuilt fromResponse.Error(message + code), mirroring the existingResponseErrorEventhandling and theResponseCompletedEventmetadata (id, createdAt, additional properties). Small, local, no exported signature changes.Test
TestResponsesStreamingFailedResponseSurfacesErrorstreams aresponse.failedevent witherror: {code, message}and asserts anErrorContentcarrying that message/code is emitted. Fails onmain(got none), passes with the fix. The existingTestResponsesStreamingResponseWithFailedUpdate_HandlesCorrectly(which only checksResponseID) still passes.