diff --git a/internal/runtime/executor/codex_executor_terminal.go b/internal/runtime/executor/codex_executor_terminal.go index ef96ed3c1..36cde2100 100644 --- a/internal/runtime/executor/codex_executor_terminal.go +++ b/internal/runtime/executor/codex_executor_terminal.go @@ -221,6 +221,9 @@ func codexTerminalFailureBody(eventData []byte) ([]byte, bool) { if len(body) == 0 { body = []byte(`{"error":{"message":"upstream stream failed without error details"}}`) } + if seq := gjson.GetBytes(eventData, "sequence_number"); seq.Exists() { + body, _ = sjson.SetBytes(body, "sequence_number", seq.Int()) + } return body, true } diff --git a/sdk/api/handlers/openai/openai_responses_handlers.go b/sdk/api/handlers/openai/openai_responses_handlers.go index c4159421a..ea5723db9 100644 --- a/sdk/api/handlers/openai/openai_responses_handlers.go +++ b/sdk/api/handlers/openai/openai_responses_handlers.go @@ -191,11 +191,19 @@ func (f *responsesSSEFramer) repairErrorPayload(payload []byte) []byte { } f.terminalEvent = failureEvent errText := responsesStreamErrorText(errMsg, status) + seq := 0 + if s := gjson.GetBytes(payload, "sequence_number"); s.Exists() { + seq = int(s.Int()) + } else if origSeq := gjson.Get(errText, "sequence_number"); origSeq.Exists() { + seq = int(origSeq.Int()) + } else if f != nil && f.dataFrames > 0 { + seq = f.dataFrames - 1 + } if failureEvent == "response.failed" { - chunk := handlers.BuildOpenAIResponsesStreamFailedChunk(status, errText, 0) + chunk := handlers.BuildOpenAIResponsesStreamFailedChunk(status, errText, seq) return []byte(fmt.Sprintf("event: response.failed\ndata: %s\n\n", chunk)) } - chunk := handlers.BuildOpenAIResponsesStreamErrorChunk(status, errText, 0) + chunk := handlers.BuildOpenAIResponsesStreamErrorChunk(status, errText, seq) return []byte(fmt.Sprintf("event: error\ndata: %s\n\n", chunk)) } @@ -817,66 +825,89 @@ func sanitizeResponsesStreamEventName(eventName string) string { return truncateResponsesStreamErrorText(redactResponsesStreamErrorText(strings.TrimSpace(eventName)), responsesStreamErrorFieldLimit) } +func isResponsesStreamSensitiveKey(key string) bool { + k := strings.ToLower(strings.TrimSpace(key)) + k = strings.ReplaceAll(k, "-", "_") + if strings.Contains(k, "tokens") || strings.Contains(k, "token_count") || strings.Contains(k, "token_limit") || strings.Contains(k, "token_usage") { + return false + } + switch k { + case "authorization", "secret", "password", "passwd", "api_key", "apikey", "token", "access_token", "refresh_token", "id_token", "auth_token", "session_token", "api_token", "client_secret", "client_key": + return true + } + return strings.HasSuffix(k, "_secret") || + strings.HasSuffix(k, "_password") || + strings.HasSuffix(k, "_api_key") || + strings.HasSuffix(k, "_token") +} + +func sanitizeResponsesStreamErrorNode(val any) any { + switch v := val.(type) { + case string: + return truncateResponsesStreamErrorText(redactResponsesStreamErrorText(v), responsesStreamErrorMessageLimit) + case map[string]any: + cleaned := make(map[string]any, len(v)) + for k, item := range v { + if isResponsesStreamSensitiveKey(k) { + cleaned[k] = "[REDACTED]" + continue + } + cleaned[k] = sanitizeResponsesStreamErrorNode(item) + } + return cleaned + case []any: + cleaned := make([]any, len(v)) + for i, item := range v { + cleaned[i] = sanitizeResponsesStreamErrorNode(item) + } + return cleaned + default: + return val + } +} + func responsesStreamErrorText(errMsg *interfaces.ErrorMessage, status int) string { text := http.StatusText(status) if errMsg != nil && errMsg.Error != nil && strings.TrimSpace(errMsg.Error.Error()) != "" { text = strings.TrimSpace(errMsg.Error.Error()) } - if !json.Valid([]byte(text)) { - return truncateResponsesStreamErrorText(redactResponsesStreamErrorText(text), responsesStreamErrorMessageLimit) + trimmed := strings.TrimSpace(text) + if !json.Valid([]byte(trimmed)) { + return truncateResponsesStreamErrorText(redactResponsesStreamErrorText(trimmed), responsesStreamErrorMessageLimit) } - root := gjson.Parse(text) - errorNode := root.Get("error") - if !errorNode.Exists() || !errorNode.IsObject() { - errorNode = root.Get("response.error") + var root map[string]any + dec := json.NewDecoder(bytes.NewReader([]byte(trimmed))) + dec.UseNumber() + if errUnmarshal := dec.Decode(&root); errUnmarshal != nil { + return truncateResponsesStreamErrorText(redactResponsesStreamErrorText(trimmed), responsesStreamErrorMessageLimit) } - if errorNode.Exists() && errorNode.IsObject() { - safe := []byte(`{"error":{}}`) - copied := false - for _, field := range []string{"type", "code", "message", "param", "retryable"} { - value := errorNode.Get(field) - if !value.Exists() || value.Type == gjson.Null { - continue - } - if field == "retryable" { - safe, _ = sjson.SetBytes(safe, "error.retryable", value.Bool()) - copied = true - continue - } - limit := responsesStreamErrorFieldLimit - if field == "message" { - limit = responsesStreamErrorMessageLimit - } - safe, _ = sjson.SetBytes(safe, "error."+field, truncateResponsesStreamErrorText(redactResponsesStreamErrorText(value.String()), limit)) - copied = true - } - if copied { - return string(safe) + + errorNode, hasError := root["error"].(map[string]any) + if !hasError { + if resp, ok := root["response"].(map[string]any); ok { + errorNode, hasError = resp["error"].(map[string]any) } } - safe := []byte(`{"type":"error"}`) - copied := false - for _, field := range []string{"code", "message", "param", "retryable"} { - value := root.Get(field) - if !value.Exists() || value.Type == gjson.Null { - continue + if hasError { + cleanedError := sanitizeResponsesStreamErrorNode(errorNode) + out := map[string]any{ + "error": cleanedError, } - if field == "retryable" { - safe, _ = sjson.SetBytes(safe, "retryable", value.Bool()) - copied = true - continue + if seq, ok := root["sequence_number"]; ok { + out["sequence_number"] = seq } - limit := responsesStreamErrorFieldLimit - if field == "message" { - limit = responsesStreamErrorMessageLimit + data, errMarshal := json.Marshal(out) + if errMarshal == nil { + return string(data) } - safe, _ = sjson.SetBytes(safe, field, truncateResponsesStreamErrorText(redactResponsesStreamErrorText(value.String()), limit)) - copied = true } - if copied { - return string(safe) + + cleanedRoot := sanitizeResponsesStreamErrorNode(root) + data, errMarshal := json.Marshal(cleanedRoot) + if errMarshal == nil { + return string(data) } return http.StatusText(status) } @@ -954,12 +985,19 @@ func (h *OpenAIResponsesAPIHandler) forwardResponsesStream(c *gin.Context, flush if framer.terminalEvent != "" { return } + seq := 0 + if framer != nil { + seq = framer.dataFrames + } + if origSeq := gjson.Get(errText, "sequence_number"); origSeq.Exists() { + seq = int(origSeq.Int()) + } if isCodexResponsesClientRequest(c) { - chunk := handlers.BuildOpenAIResponsesStreamFailedChunk(status, errText, 0) + chunk := handlers.BuildOpenAIResponsesStreamFailedChunk(status, errText, seq) _, _ = fmt.Fprintf(c.Writer, "\nevent: response.failed\ndata: %s\n\n", string(chunk)) return } - chunk := handlers.BuildOpenAIResponsesStreamErrorChunk(status, errText, 0) + chunk := handlers.BuildOpenAIResponsesStreamErrorChunk(status, errText, seq) _, _ = fmt.Fprintf(c.Writer, "\nevent: error\ndata: %s\n\n", string(chunk)) } diff --git a/sdk/api/handlers/openai/openai_responses_handlers_stream_error_test.go b/sdk/api/handlers/openai/openai_responses_handlers_stream_error_test.go index 95e189e4d..496807739 100644 --- a/sdk/api/handlers/openai/openai_responses_handlers_stream_error_test.go +++ b/sdk/api/handlers/openai/openai_responses_handlers_stream_error_test.go @@ -521,9 +521,12 @@ func TestForwardResponsesStreamExposesTerminalErrors(t *testing.T) { if exposed != tc.wantExposed { t.Fatalf("error exposed = %t, want %t: %q", exposed, tc.wantExposed, body) } - if exposed && strings.Contains(body, `"error":{`) { + if exposed && !strings.Contains(body, "event: error\ndata: ") { t.Fatalf("expected streaming error chunk, got HTTP error body: %q", body) } + if exposed && !strings.Contains(body, `"error":{`) { + t.Fatalf("expected nested error in streaming error chunk, got: %q", body) + } }) } } diff --git a/sdk/api/handlers/openai/openai_responses_handlers_stream_test.go b/sdk/api/handlers/openai/openai_responses_handlers_stream_test.go index 80c648748..b0ca07b9a 100644 --- a/sdk/api/handlers/openai/openai_responses_handlers_stream_test.go +++ b/sdk/api/handlers/openai/openai_responses_handlers_stream_test.go @@ -2,6 +2,8 @@ package openai import ( "bytes" + "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -312,3 +314,215 @@ func TestForwardResponsesStreamDropsIncompleteTrailingDataChunkOnFlush(t *testin t.Fatalf("unterminated framing test stream did not end with an error: %q", got) } } + +func TestForwardResponsesStreamErrorEventPreservesNestedError(t *testing.T) { + h, recorder, c, flusher := newResponsesStreamTestHandler(t) + c.Request.Header.Set("User-Agent", "codex_vscode/0.153.4 (Ubuntu 22.4.0; x86_64)") + + data := make(chan []byte) + errs := make(chan *interfaces.ErrorMessage, 1) + + go func() { + data <- []byte("event: response.created\ndata: {\"type\":\"response.created\",\"sequence_number\":0}\n\n") + data <- []byte("event: response.in_progress\ndata: {\"type\":\"response.in_progress\",\"sequence_number\":1}\n\n") + close(data) + + errText := `{"error":{"type":"invalid_request","code":"cyber_policy","message":"This content was flagged for possible cybersecurity risk. If this seems wrong, try rephrasing your request. To get authorized for security work, join the Trusted Access for Cyber program: https://chatgpt.com/cyber","param":null}}` + errs <- &interfaces.ErrorMessage{ + StatusCode: http.StatusBadRequest, + Error: errors.New(errText), + } + close(errs) + }() + + framer := &responsesSSEFramer{} + h.forwardResponsesStream(c, flusher, func(error) {}, data, errs, framer) + + body := recorder.Body.String() + if !strings.Contains(body, "event: error\n") { + t.Fatalf("expected event: error in output, got: %q", body) + } + + parts := strings.Split(strings.TrimSpace(body), "\n\n") + if len(parts) < 3 { + t.Fatalf("expected at least 3 SSE events, got %d. Body: %q", len(parts), body) + } + + lastPart := strings.TrimSpace(parts[len(parts)-1]) + if !strings.HasPrefix(lastPart, "event: error\ndata: ") { + t.Fatalf("last event is not error event: %q", lastPart) + } + + jsonPayload := strings.TrimSpace(strings.TrimPrefix(lastPart, "event: error\ndata: ")) + var payload struct { + Type string `json:"type"` + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` + } + if errUnmarshal := json.Unmarshal([]byte(jsonPayload), &payload); errUnmarshal != nil { + t.Fatalf("unmarshal error event payload: %v (raw: %s)", errUnmarshal, jsonPayload) + } + if payload.Type != "error" { + t.Fatalf("payload.Type = %q, want error", payload.Type) + } + if payload.SequenceNumber != 2 { + t.Fatalf("payload.SequenceNumber = %d, want 2", payload.SequenceNumber) + } + if payload.Error["type"] != "invalid_request" { + t.Fatalf("payload.Error.type = %v, want invalid_request", payload.Error["type"]) + } + if payload.Error["code"] != "cyber_policy" { + t.Fatalf("payload.Error.code = %v, want cyber_policy", payload.Error["code"]) + } + if param, exists := payload.Error["param"]; !exists || param != nil { + t.Fatalf("payload.Error.param = %v, want null/nil", param) + } +} + +func TestResponsesSSEFramerRepairErrorPayloadCalculatesCorrectSequenceNumber(t *testing.T) { + // Case 1: First frame is an error without sequence_number -> should be 0 + framer1 := &responsesSSEFramer{} + var out1 bytes.Buffer + framer1.WriteChunk(&out1, []byte("event: error\ndata: {\"error\":{\"code\":\"bad_request\"}}\n\n")) + var p1 struct { + Type string `json:"type"` + SequenceNumber int `json:"sequence_number"` + Error map[string]any `json:"error"` + } + payload1 := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(out1.String()), "event: error\ndata: ")) + if err := json.Unmarshal([]byte(payload1), &p1); err != nil { + t.Fatalf("unmarshal error payload: %v", err) + } + if p1.SequenceNumber != 0 { + t.Fatalf("first frame error sequence_number = %d, want 0", p1.SequenceNumber) + } + + // Case 2: Two data frames then an error frame without sequence_number -> should be 2 + framer2 := &responsesSSEFramer{} + var out2 bytes.Buffer + framer2.WriteChunk(&out2, []byte("event: response.created\ndata: {\"type\":\"response.created\",\"sequence_number\":0}\n\n")) + framer2.WriteChunk(&out2, []byte("event: response.in_progress\ndata: {\"type\":\"response.in_progress\",\"sequence_number\":1}\n\n")) + out2.Reset() + framer2.WriteChunk(&out2, []byte("event: error\ndata: {\"error\":{\"code\":\"cyber_policy\"}}\n\n")) + var p2 struct { + Type string `json:"type"` + SequenceNumber int `json:"sequence_number"` + Error map[string]any `json:"error"` + } + payload2 := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(out2.String()), "event: error\ndata: ")) + if err := json.Unmarshal([]byte(payload2), &p2); err != nil { + t.Fatalf("unmarshal error payload: %v", err) + } + if p2.SequenceNumber != 2 { + t.Fatalf("third frame error sequence_number = %d, want 2", p2.SequenceNumber) + } +} + +func TestForwardResponsesStreamErrorEventPreservesExplicitSequenceNumber(t *testing.T) { + h, recorder, c, flusher := newResponsesStreamTestHandler(t) + c.Request.Header.Set("User-Agent", "codex_vscode/0.153.4") + + data := make(chan []byte) + errs := make(chan *interfaces.ErrorMessage, 1) + + go func() { + close(data) + errText := `{"error":{"type":"invalid_request","code":"custom"},"sequence_number":9}` + errs <- &interfaces.ErrorMessage{ + StatusCode: http.StatusBadRequest, + Error: errors.New(errText), + } + close(errs) + }() + + framer := &responsesSSEFramer{} + h.forwardResponsesStream(c, flusher, func(error) {}, data, errs, framer) + + body := recorder.Body.String() + var p struct { + Type string `json:"type"` + SequenceNumber int `json:"sequence_number"` + Error map[string]any `json:"error"` + } + lastPart := strings.TrimSpace(strings.Split(strings.TrimSpace(body), "\n\n")[0]) + payload := strings.TrimSpace(strings.TrimPrefix(lastPart, "event: error\ndata: ")) + if err := json.Unmarshal([]byte(payload), &p); err != nil { + t.Fatalf("unmarshal error payload: %v (raw: %s)", err, payload) + } + if p.SequenceNumber != 9 { + t.Fatalf("explicit sequence_number = %d, want 9", p.SequenceNumber) + } +} + +func TestResponsesStreamErrorTextSanitizesNestedSensitiveFieldsAndDotKeys(t *testing.T) { + inputJSON := `{"error":{"metadata":{"message":"Bearer test-secret"},"vendor.detail":"Bearer test-secret","access_token":"test-secret","normal_key":"safe_value"},"sequence_number":3}` + errMsg := &interfaces.ErrorMessage{ + StatusCode: http.StatusBadRequest, + Error: errors.New(inputJSON), + } + out := responsesStreamErrorText(errMsg, http.StatusBadRequest) + + var parsed struct { + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` + } + if errUnmarshal := json.Unmarshal([]byte(out), &parsed); errUnmarshal != nil { + t.Fatalf("unmarshal error text output: %v (raw: %s)", errUnmarshal, out) + } + + if parsed.SequenceNumber != 3 { + t.Fatalf("sequence_number = %d, want 3", parsed.SequenceNumber) + } + if parsed.Error["access_token"] != "[REDACTED]" { + t.Fatalf("access_token was not redacted: %v", parsed.Error["access_token"]) + } + if parsed.Error["vendor.detail"] != "Bearer [REDACTED]" { + t.Fatalf("vendor.detail was not properly redacted without path corruption: %v", parsed.Error["vendor.detail"]) + } + meta, ok := parsed.Error["metadata"].(map[string]any) + if !ok { + t.Fatalf("metadata is not a map: %v", parsed.Error["metadata"]) + } + if meta["message"] != "Bearer [REDACTED]" { + t.Fatalf("nested metadata.message was not redacted: %v", meta["message"]) + } + if parsed.Error["normal_key"] != "safe_value" { + t.Fatalf("normal_key = %v, want safe_value", parsed.Error["normal_key"]) + } +} + +func TestResponsesStreamErrorTextPreservesTokenCountersAndLargeInts(t *testing.T) { + inputJSON := `{"error":{"input_tokens":42,"token_limit":8192,"request_id":9007199254740993,"access_token":"secret123"}}` + errMsg := &interfaces.ErrorMessage{ + StatusCode: http.StatusBadRequest, + Error: errors.New(inputJSON), + } + out := responsesStreamErrorText(errMsg, http.StatusBadRequest) + + // Verify exact raw representation doesn't lose precision for 9007199254740993 + if !strings.Contains(out, "9007199254740993") { + t.Fatalf("large integer precision was lost: %s", out) + } + if strings.Contains(out, "9007199254740992") { + t.Fatalf("large integer was corrupted by float64 conversion: %s", out) + } + + var parsed struct { + Error map[string]any `json:"error"` + } + dec := json.NewDecoder(strings.NewReader(out)) + dec.UseNumber() + if errUnmarshal := dec.Decode(&parsed); errUnmarshal != nil { + t.Fatalf("unmarshal error: %v", errUnmarshal) + } + + if parsed.Error["access_token"] != "[REDACTED]" { + t.Fatalf("access_token should be redacted: %v", parsed.Error["access_token"]) + } + if parsed.Error["input_tokens"] != json.Number("42") { + t.Fatalf("input_tokens should remain number 42, got %v", parsed.Error["input_tokens"]) + } + if parsed.Error["token_limit"] != json.Number("8192") { + t.Fatalf("token_limit should remain number 8192, got %v", parsed.Error["token_limit"]) + } +} diff --git a/sdk/api/handlers/openai_responses_stream_error.go b/sdk/api/handlers/openai_responses_stream_error.go index a3c3c7e32..d2951f1fa 100644 --- a/sdk/api/handlers/openai_responses_stream_error.go +++ b/sdk/api/handlers/openai_responses_stream_error.go @@ -1,6 +1,7 @@ package handlers import ( + "bytes" "encoding/json" "fmt" "net/http" @@ -8,10 +9,9 @@ import ( ) type openAIResponsesStreamErrorChunk struct { - Type string `json:"type"` - Code string `json:"code"` - Message string `json:"message"` - SequenceNumber int `json:"sequence_number"` + Type string `json:"type"` + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` } type openAIResponsesStreamFailedChunk struct { @@ -48,11 +48,14 @@ func openAIResponsesStreamErrorCode(status int) string { } } +func unmarshalJSONWithNumber(data []byte, v any) error { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + return dec.Decode(v) +} + // BuildOpenAIResponsesStreamErrorChunk builds an OpenAI Responses streaming error chunk. -// -// Important: OpenAI's HTTP error bodies are shaped like {"error":{...}}; those are valid for -// non-streaming responses, but streaming clients validate SSE `data:` payloads against a union -// of chunks that requires a top-level `type` field. +// It matches the official responses streaming event shape where the error details are nested. func BuildOpenAIResponsesStreamErrorChunk(status int, errText string, sequenceNumber int) []byte { if status <= 0 { status = http.StatusInternalServerError @@ -71,32 +74,15 @@ func BuildOpenAIResponsesStreamErrorChunk(status int, errText string, sequenceNu trimmed := strings.TrimSpace(errText) if trimmed != "" && json.Valid([]byte(trimmed)) { var payload map[string]any - if err := json.Unmarshal([]byte(trimmed), &payload); err == nil { - if t, ok := payload["type"].(string); ok && strings.TrimSpace(t) == "error" { - if m, ok := payload["message"].(string); ok && strings.TrimSpace(m) != "" { - message = strings.TrimSpace(m) - } - if v, ok := payload["code"]; ok && v != nil { - if c, ok := v.(string); ok && strings.TrimSpace(c) != "" { - code = strings.TrimSpace(c) - } else { - code = strings.TrimSpace(fmt.Sprint(v)) - } - } - if v, ok := payload["sequence_number"].(float64); ok && sequenceNumber == 0 { - sequenceNumber = int(v) - } - } - if e, ok := payload["error"].(map[string]any); ok { - if m, ok := e["message"].(string); ok && strings.TrimSpace(m) != "" { - message = strings.TrimSpace(m) - } - if v, ok := e["code"]; ok && v != nil { - if c, ok := v.(string); ok && strings.TrimSpace(c) != "" { - code = strings.TrimSpace(c) - } else { - code = strings.TrimSpace(fmt.Sprint(v)) + if errUnmarshal := unmarshalJSONWithNumber([]byte(trimmed), &payload); errUnmarshal == nil { + if v, ok := payload["sequence_number"]; ok { + switch n := v.(type) { + case json.Number: + if seqInt, err := n.Int64(); err == nil { + sequenceNumber = int(seqInt) } + case float64: + sequenceNumber = int(n) } } } @@ -106,39 +92,58 @@ func BuildOpenAIResponsesStreamErrorChunk(status int, errText string, sequenceNu code = "unknown_error" } - data, err := json.Marshal(openAIResponsesStreamErrorChunk{ + errorDetail := openAIResponsesStreamErrorDetail(status, errText, code, message) + + data, errMarshal := json.Marshal(openAIResponsesStreamErrorChunk{ Type: "error", - Code: code, - Message: message, + Error: errorDetail, SequenceNumber: sequenceNumber, }) - if err == nil { + if errMarshal == nil { return data } // Extremely defensive fallback. + fallbackDetail := map[string]any{ + "type": "server_error", + "code": "internal_server_error", + "message": message, + "param": nil, + } data, _ = json.Marshal(openAIResponsesStreamErrorChunk{ Type: "error", - Code: "internal_server_error", - Message: message, + Error: fallbackDetail, SequenceNumber: sequenceNumber, }) if len(data) > 0 { return data } - return []byte(`{"type":"error","code":"internal_server_error","message":"internal error","sequence_number":0}`) + return []byte(`{"type":"error","error":{"type":"server_error","code":"internal_server_error","message":"internal error","param":null},"sequence_number":0}`) } -func openAIResponsesStreamFailedErrorDetail(status int, errText, code, message string) map[string]any { +func openAIResponsesStreamErrorDetail(status int, errText, code, message string) map[string]any { var payload map[string]any - if errUnmarshal := json.Unmarshal([]byte(strings.TrimSpace(errText)), &payload); errUnmarshal == nil { - if errorDetail, ok := payload["error"].(map[string]any); ok { - return errorDetail - } - if response, ok := payload["response"].(map[string]any); ok { - if errorDetail, ok := response["error"].(map[string]any); ok { + trimmed := strings.TrimSpace(errText) + if trimmed != "" && json.Valid([]byte(trimmed)) { + if errUnmarshal := unmarshalJSONWithNumber([]byte(trimmed), &payload); errUnmarshal == nil { + if errorDetail, ok := payload["error"].(map[string]any); ok { return errorDetail } + if response, ok := payload["response"].(map[string]any); ok { + if errorDetail, ok := response["error"].(map[string]any); ok { + return errorDetail + } + } + if m, ok := payload["message"].(string); ok && strings.TrimSpace(m) != "" { + message = strings.TrimSpace(m) + } + if v, ok := payload["code"]; ok && v != nil { + if c, ok := v.(string); ok && strings.TrimSpace(c) != "" { + code = strings.TrimSpace(c) + } else { + code = strings.TrimSpace(fmt.Sprint(v)) + } + } } } @@ -146,11 +151,25 @@ func openAIResponsesStreamFailedErrorDetail(status int, errText, code, message s if status >= http.StatusInternalServerError { errorType = "server_error" } - return map[string]any{ + detail := map[string]any{ "type": errorType, "code": code, "message": message, + "param": nil, } + if payload != nil { + if t, ok := payload["type"].(string); ok && strings.TrimSpace(t) != "" && strings.TrimSpace(t) != "error" { + detail["type"] = strings.TrimSpace(t) + } + if paramVal, exists := payload["param"]; exists { + detail["param"] = paramVal + } + } + return detail +} + +func openAIResponsesStreamFailedErrorDetail(status int, errText, code, message string) map[string]any { + return openAIResponsesStreamErrorDetail(status, errText, code, message) } // BuildOpenAIResponsesStreamFailedChunk builds the terminal Responses event used by official Codex clients. @@ -163,15 +182,19 @@ func BuildOpenAIResponsesStreamFailedChunk(status int, errText string, sequenceN sequenceNumber = 0 } - legacyChunk := BuildOpenAIResponsesStreamErrorChunk(status, errText, sequenceNumber) - var legacyPayload openAIResponsesStreamErrorChunk - if errUnmarshal := json.Unmarshal(legacyChunk, &legacyPayload); errUnmarshal != nil { - legacyPayload.Code = openAIResponsesStreamErrorCode(status) - legacyPayload.Message = http.StatusText(status) - legacyPayload.SequenceNumber = sequenceNumber - } - if sequenceNumber == 0 && legacyPayload.SequenceNumber > 0 { - sequenceNumber = legacyPayload.SequenceNumber + errorChunkBytes := BuildOpenAIResponsesStreamErrorChunk(status, errText, sequenceNumber) + var errorChunk openAIResponsesStreamErrorChunk + _ = unmarshalJSONWithNumber(errorChunkBytes, &errorChunk) + sequenceNumber = errorChunk.SequenceNumber + + errorDetail := errorChunk.Error + if errorDetail == nil { + code := openAIResponsesStreamErrorCode(status) + message := strings.TrimSpace(errText) + if message == "" { + message = http.StatusText(status) + } + errorDetail = openAIResponsesStreamErrorDetail(status, errText, code, message) } data, errMarshal := json.Marshal(openAIResponsesStreamFailedChunk{ @@ -179,7 +202,7 @@ func BuildOpenAIResponsesStreamFailedChunk(status int, errText string, sequenceN SequenceNumber: sequenceNumber, Response: openAIResponsesStreamFailedResponse{ Status: "failed", - Error: openAIResponsesStreamFailedErrorDetail(status, errText, legacyPayload.Code, legacyPayload.Message), + Error: errorDetail, }, }) if errMarshal == nil { diff --git a/sdk/api/handlers/openai_responses_stream_error_test.go b/sdk/api/handlers/openai_responses_stream_error_test.go index c6dfd25ef..01752090f 100644 --- a/sdk/api/handlers/openai_responses_stream_error_test.go +++ b/sdk/api/handlers/openai_responses_stream_error_test.go @@ -3,6 +3,7 @@ package handlers import ( "encoding/json" "net/http" + "strings" "testing" ) @@ -15,11 +16,15 @@ func TestBuildOpenAIResponsesStreamErrorChunk(t *testing.T) { if payload["type"] != "error" { t.Fatalf("type = %v, want %q", payload["type"], "error") } - if payload["code"] != "internal_server_error" { - t.Fatalf("code = %v, want %q", payload["code"], "internal_server_error") + errorObj, ok := payload["error"].(map[string]any) + if !ok { + t.Fatalf("error is not an object: %v", payload["error"]) } - if payload["message"] != "unexpected EOF" { - t.Fatalf("message = %v, want %q", payload["message"], "unexpected EOF") + if errorObj["code"] != "internal_server_error" { + t.Fatalf("code = %v, want %q", errorObj["code"], "internal_server_error") + } + if errorObj["message"] != "unexpected EOF" { + t.Fatalf("message = %v, want %q", errorObj["message"], "unexpected EOF") } if payload["sequence_number"] != float64(0) { t.Fatalf("sequence_number = %v, want %v", payload["sequence_number"], 0) @@ -39,11 +44,105 @@ func TestBuildOpenAIResponsesStreamErrorChunkExtractsHTTPErrorBody(t *testing.T) if payload["type"] != "error" { t.Fatalf("type = %v, want %q", payload["type"], "error") } - if payload["code"] != "internal_server_error" { - t.Fatalf("code = %v, want %q", payload["code"], "internal_server_error") + errorObj, ok := payload["error"].(map[string]any) + if !ok { + t.Fatalf("error is not an object: %v", payload["error"]) } - if payload["message"] != "oops" { - t.Fatalf("message = %v, want %q", payload["message"], "oops") + if errorObj["code"] != "internal_server_error" { + t.Fatalf("code = %v, want %q", errorObj["code"], "internal_server_error") + } + if errorObj["message"] != "oops" { + t.Fatalf("message = %v, want %q", errorObj["message"], "oops") + } + if errorObj["type"] != "server_error" { + t.Fatalf("error.type = %v, want %q", errorObj["type"], "server_error") + } +} + +func TestBuildOpenAIResponsesStreamErrorChunkPreservesNestedError(t *testing.T) { + errText := `{"error":{"type":"invalid_request","code":"cyber_policy","message":"This content was flagged for possible cybersecurity risk.","param":null}}` + chunk := BuildOpenAIResponsesStreamErrorChunk(http.StatusBadRequest, errText, 2) + var payload struct { + Type string `json:"type"` + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` + } + if err := json.Unmarshal(chunk, &payload); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if payload.Type != "error" { + t.Fatalf("type = %q, want %q", payload.Type, "error") + } + if payload.SequenceNumber != 2 { + t.Fatalf("sequence_number = %d, want 2", payload.SequenceNumber) + } + if payload.Error["type"] != "invalid_request" { + t.Fatalf("error.type = %v, want invalid_request", payload.Error["type"]) + } + if payload.Error["code"] != "cyber_policy" { + t.Fatalf("error.code = %v, want cyber_policy", payload.Error["code"]) + } + if payload.Error["message"] != "This content was flagged for possible cybersecurity risk." { + t.Fatalf("error.message = %v", payload.Error["message"]) + } + if param, exists := payload.Error["param"]; !exists || param != nil { + t.Fatalf("error.param = %v, want nil", param) + } +} + +func TestBuildOpenAIResponsesStreamErrorChunkPreservesCustomAndEmptyFields(t *testing.T) { + // Preserves empty error object {} + emptyChunk := BuildOpenAIResponsesStreamErrorChunk(http.StatusBadRequest, `{"error":{}}`, 0) + var emptyPayload struct { + Type string `json:"type"` + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` + } + if err := json.Unmarshal(emptyChunk, &emptyPayload); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(emptyPayload.Error) != 0 { + t.Fatalf("expected empty error object, got %v", emptyPayload.Error) + } + + // Preserves custom fields and types without dropping + customText := `{"error":{"type":"custom_type","code":"custom_code","custom_key":"custom_val","is_flag":true,"count":42}}` + customChunk := BuildOpenAIResponsesStreamErrorChunk(http.StatusBadRequest, customText, 5) + var customPayload struct { + Type string `json:"type"` + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` + } + if err := json.Unmarshal(customChunk, &customPayload); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if customPayload.SequenceNumber != 5 { + t.Fatalf("sequence_number = %d, want 5", customPayload.SequenceNumber) + } + if customPayload.Error["custom_key"] != "custom_val" { + t.Fatalf("custom_key = %v, want custom_val", customPayload.Error["custom_key"]) + } + if customPayload.Error["is_flag"] != true { + t.Fatalf("is_flag = %v, want true", customPayload.Error["is_flag"]) + } + if customPayload.Error["count"] != float64(42) { + t.Fatalf("count = %v, want 42", customPayload.Error["count"]) + } +} + +func TestBuildOpenAIResponsesStreamErrorChunkPrioritizesPayloadSequenceNumber(t *testing.T) { + errText := `{"error":{"type":"invalid_request","code":"blocked"},"sequence_number":7}` + chunk := BuildOpenAIResponsesStreamErrorChunk(http.StatusBadRequest, errText, 2) + var payload struct { + Type string `json:"type"` + Error map[string]any `json:"error"` + SequenceNumber int `json:"sequence_number"` + } + if err := json.Unmarshal(chunk, &payload); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if payload.SequenceNumber != 7 { + t.Fatalf("sequence_number = %d, want 7 (from payload)", payload.SequenceNumber) } } @@ -88,3 +187,43 @@ func TestBuildOpenAIResponsesStreamFailedChunkPreservesNestedError(t *testing.T) t.Fatalf("response.error.message = %q, want %q", payload.Response.Error.Message, "blocked") } } + +func TestBuildOpenAIResponsesStreamFailedChunkPrioritizesPayloadSequenceNumber(t *testing.T) { + chunk := BuildOpenAIResponsesStreamFailedChunk( + http.StatusBadRequest, + `{"error":{"type":"invalid_request","code":"cyber_policy","message":"blocked"},"sequence_number":7}`, + 2, + ) + + var payload struct { + Type string `json:"type"` + SequenceNumber int `json:"sequence_number"` + } + if err := json.Unmarshal(chunk, &payload); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if payload.SequenceNumber != 7 { + t.Fatalf("sequence_number = %d, want 7 (from payload over arg 2)", payload.SequenceNumber) + } +} + +func TestBuildOpenAIResponsesStreamErrorChunkPreservesLargeIntPrecision(t *testing.T) { + errText := `{"error":{"type":"invalid_request","code":"blocked","request_id":9007199254740993}}` + chunk := BuildOpenAIResponsesStreamErrorChunk(http.StatusBadRequest, errText, 0) + raw := string(chunk) + if !strings.Contains(raw, "9007199254740993") { + t.Fatalf("large integer precision was lost in error chunk: %s", raw) + } + if strings.Contains(raw, "9007199254740992") { + t.Fatalf("large integer was corrupted by float64 in error chunk: %s", raw) + } + + failedChunk := BuildOpenAIResponsesStreamFailedChunk(http.StatusBadRequest, errText, 0) + failedRaw := string(failedChunk) + if !strings.Contains(failedRaw, "9007199254740993") { + t.Fatalf("large integer precision was lost in failed chunk: %s", failedRaw) + } + if strings.Contains(failedRaw, "9007199254740992") { + t.Fatalf("large integer was corrupted by float64 in failed chunk: %s", failedRaw) + } +}