diff --git a/sdk/api/handlers/openai/openai_responses_websocket_forward.go b/sdk/api/handlers/openai/openai_responses_websocket_forward.go index 1333ebffdcd..24e60e127e0 100644 --- a/sdk/api/handlers/openai/openai_responses_websocket_forward.go +++ b/sdk/api/handlers/openai/openai_responses_websocket_forward.go @@ -77,6 +77,36 @@ func (h *OpenAIResponsesAPIHandler) forwardResponsesWebsocket( } localFrames := opts.localInterrupt.framesChan() + // handleErr ends the turn on a terminal stream error (non-nil errMsg). + handleErr := func(errMsg *interfaces.ErrorMessage) ([]byte, string, []string, *interfaces.ErrorMessage, error) { + h.LoggingAPIResponseError(context.WithValue(context.Background(), "gin", c), errMsg) + if opts.suppressError != nil && opts.suppressError(errMsg) { + cancel(errMsg.Error) + return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, nil + } + markAPIResponseTimestamp(c) + if matched, errClose := writer.closeForUpstreamError(errMsg.Error); matched { + cancel(errMsg.Error) + if errClose != nil { + return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, errClose + } + return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, websocket.ErrCloseSent + } + + errorPayload, wrote, errTerminate := writeResponsesWebsocketTerminalError(writer, wsTimelineLog, errMsg, nil) + if wrote { + log.Infof( + "responses websocket: downstream_out id=%s type=%d event=%s payload=%s", + sessionID, + websocket.TextMessage, + websocketPayloadEventType(errorPayload), + websocketPayloadPreview(errorPayload), + ) + } + cancel(errMsg.Error) + return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, errTerminate + } + for { select { case interruptPayload := <-localFrames: @@ -98,36 +128,27 @@ func (h *OpenAIResponsesAPIHandler) forwardResponsesWebsocket( cancel(nil) return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), nil, nil } - - h.LoggingAPIResponseError(context.WithValue(context.Background(), "gin", c), errMsg) - if opts.suppressError != nil && opts.suppressError(errMsg) { - cancel(errMsg.Error) - return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, nil - } - markAPIResponseTimestamp(c) - if matched, errClose := writer.closeForUpstreamError(errMsg.Error); matched { - cancel(errMsg.Error) - if errClose != nil { - return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, errClose - } - return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, websocket.ErrCloseSent - } - - errorPayload, wrote, errTerminate := writeResponsesWebsocketTerminalError(writer, wsTimelineLog, errMsg, nil) - if wrote { - log.Infof( - "responses websocket: downstream_out id=%s type=%d event=%s payload=%s", - sessionID, - websocket.TextMessage, - websocketPayloadEventType(errorPayload), - websocketPayloadPreview(errorPayload), - ) - } - cancel(errMsg.Error) - return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, errTerminate + return handleErr(errMsg) case chunk, ok := <-data: if !ok { - if opts.duplexStream != nil && opts.duplexStream() { + duplex := opts.duplexStream != nil && opts.duplexStream() + if duplex || !completed { + // The stream producers send a terminal error before they close errs and then + // data, so it is already buffered here. The select picks between the closed + // data channel and that error at random; take the error. Otherwise a pinned-auth + // failure that needs a replay close (1012) ends as a plain disconnect (1006), + // and a duplex stream loses the error its executor reported. + if errs != nil { + select { + case pendingErrMsg, okErr := <-errs: + if okErr && pendingErrMsg != nil { + return handleErr(pendingErrMsg) + } + default: + } + } + } + if duplex { // A duplex stream ends with its socket, not an individual response. // The data channel may close before select observes its final error. _, errClose := writer.closeWithoutError() diff --git a/sdk/api/handlers/openai/openai_responses_websocket_test.go b/sdk/api/handlers/openai/openai_responses_websocket_test.go index a36b930f970..20de08f8153 100644 --- a/sdk/api/handlers/openai/openai_responses_websocket_test.go +++ b/sdk/api/handlers/openai/openai_responses_websocket_test.go @@ -7976,3 +7976,32 @@ func TestResponsesWebsocketReauthorizesEachTurnAndKeepsSocketOpen(t *testing.T) } } } + +// The stream producers buffer a terminal error, then close errs and data. forwardResponsesWebsocket +// must report that error, not "stream closed before response.completed", whichever channel its +// select reads first; otherwise a pinned-auth failure loses its replay close. +func TestForwardResponsesWebsocketPrefersBufferedErrorOverClosedData(t *testing.T) { + gin.SetMode(gin.TestMode) + h := NewOpenAIResponsesAPIHandler(handlers.NewBaseAPIHandlers(&sdkconfig.SDKConfig{}, nil)) + want := &interfaces.ErrorMessage{StatusCode: http.StatusUnauthorized, Error: errors.New("credential failed")} + for _, duplex := range []bool{false, true} { + for i := 0; i < 200; i++ { + ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx.Request = httptest.NewRequest(http.MethodGet, "/v1/responses/ws", nil) + data := make(chan []byte) + errCh := make(chan *interfaces.ErrorMessage, 1) + errCh <- want + close(errCh) + close(data) + _, _, _, errMsg, errForward := h.forwardResponsesWebsocket(ctx, newResponsesWebsocketWriter(nil), func(...interface{}) {}, + data, errCh, nil, "session-1", + responsesWebsocketForwardOptions{ + suppressError: func(*interfaces.ErrorMessage) bool { return true }, + duplexStream: func() bool { return duplex }, + }) + if errMsg != want || errForward != nil { + t.Fatalf("duplex=%v iteration %d: errMsg = %v, err = %v; want the buffered credential error", duplex, i, errMsg, errForward) + } + } + } +}