From 171746357a081b6d95a8eac1b4a948c9c808f044 Mon Sep 17 00:00:00 2001 From: Phung Quang Minh Date: Tue, 27 Oct 2026 23:17:41 +0700 Subject: [PATCH] fix(extproc): encode user-facing errors as JSON Fixes theagentrouter/agent-router#2926 Signed-off-by: Phung Quang Minh --- internal/extproc/processor_impl.go | 23 +++- internal/extproc/processor_impl_test.go | 159 ++++++++++++++++++++++++ tests/data-plane/testupstream_test.go | 45 +++++++ 3 files changed, 225 insertions(+), 2 deletions(-) diff --git a/internal/extproc/processor_impl.go b/internal/extproc/processor_impl.go index d6f9929b2c..c9ab3f9852 100644 --- a/internal/extproc/processor_impl.go +++ b/internal/extproc/processor_impl.go @@ -194,8 +194,27 @@ func (r *routerProcessor[ReqT, RespT, RespChunkT, EndpointSpecT]) ProcessRespons // formatUserFacingErrorJSON formats a user-facing error as a JSON response body. // Returns JSON in format: {"type":"error","error":{"type":"","code":"","message":""}} func formatUserFacingErrorJSON(errorType string, statusCode int, message string) []byte { - return fmt.Appendf(nil, `{"type":"error","error":{"type":"%s","code":"%d","message":"%s"}}`, - errorType, statusCode, message) + // The fixed string-only envelope has no values that the JSON encoder cannot marshal. + body, _ := json.Marshal(struct { + Type string `json:"type"` + Error struct { + Type string `json:"type"` + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + }{ + Type: "error", + Error: struct { + Type string `json:"type"` + Code string `json:"code"` + Message string `json:"message"` + }{ + Type: errorType, + Code: strconv.Itoa(statusCode), + Message: message, + }, + }) + return body } // respondLocally answers the request from the gateway itself instead of dispatching it upstream. diff --git a/internal/extproc/processor_impl_test.go b/internal/extproc/processor_impl_test.go index 441edae767..5a1b413fb7 100644 --- a/internal/extproc/processor_impl_test.go +++ b/internal/extproc/processor_impl_test.go @@ -14,6 +14,7 @@ import ( "io" "log/slog" "mime/multipart" + "strconv" "testing" "unsafe" @@ -24,6 +25,7 @@ import ( "github.com/google/cel-go/cel" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/propagation" + "google.golang.org/grpc/codes" "google.golang.org/protobuf/types/known/structpb" anthropicschema "github.com/envoyproxy/ai-gateway/internal/apischema/anthropic" @@ -75,6 +77,133 @@ func TestNewFactory(t *testing.T) { }) } +type testUserFacingErrorEnvelope struct { + Type string `json:"type"` + Error testUserFacingErrorDetails `json:"error"` +} + +type testUserFacingErrorDetails struct { + Type string `json:"type"` + Code string `json:"code"` + Message string `json:"message"` +} + +func Test_formatUserFacingErrorJSON(t *testing.T) { + tests := []struct { + name string + errorType string + statusCode int + message string + wantJSON string + }{ + { + name: "ordinary text", + errorType: "BadRequest", + statusCode: 400, + message: "request failed", + wantJSON: `{"type":"error","error":{"type":"BadRequest","code":"400","message":"request failed"}}`, + }, + { + name: "quoted type and message", + errorType: `Bad"Request`, + statusCode: 400, + message: `message says "hello"`, + wantJSON: `{"type":"error","error":{"type":"Bad\"Request","code":"400","message":"message says \"hello\""}}`, + }, + { + name: "backslashes", + errorType: `Bad\Request`, + statusCode: 422, + message: `path C:\temp`, + wantJSON: `{"type":"error","error":{"type":"Bad\\Request","code":"422","message":"path C:\\temp"}}`, + }, + { + name: "newline tab and NUL characters", + errorType: "BadRequest", + statusCode: 400, + message: "line one\nline two\ttab\x00nul", + wantJSON: `{"type":"error","error":{"type":"BadRequest","code":"400","message":"line one\nline two\ttab\u0000nul"}}`, + }, + { + name: "Unicode", + errorType: "BadRequest", + statusCode: 422, + message: "temperature 雪 🌡️", + wantJSON: `{"type":"error","error":{"type":"BadRequest","code":"422","message":"temperature 雪 🌡️"}}`, + }, + { + name: "empty strings", + errorType: "", + statusCode: 400, + message: "", + wantJSON: `{"type":"error","error":{"type":"","code":"400","message":""}}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + body := formatUserFacingErrorJSON(tt.errorType, tt.statusCode, tt.message) + require.JSONEq(t, tt.wantJSON, string(body)) + + var got testUserFacingErrorEnvelope + require.NoError(t, json.Unmarshal(body, &got)) + require.Equal(t, testUserFacingErrorEnvelope{ + Type: "error", + Error: testUserFacingErrorDetails{ + Type: tt.errorType, + Code: strconv.Itoa(tt.statusCode), + Message: tt.message, + }, + }, got) + }) + } +} + +func Test_createUserFacingErrorResponse(t *testing.T) { + for _, tt := range []struct { + statusCode int + wantHTTPStatus typev3.StatusCode + }{ + {statusCode: 400, wantHTTPStatus: typev3.StatusCode(400)}, + {statusCode: 422, wantHTTPStatus: typev3.StatusCode(422)}, + } { + t.Run(strconv.Itoa(tt.statusCode), func(t *testing.T) { + message := `temperature "bad" at 20°C ☃` + response := createUserFacingErrorResponse(tt.statusCode, "BadRequest", message) + immediateResponse, ok := response.Response.(*extprocv3.ProcessingResponse_ImmediateResponse) + require.True(t, ok) + require.NotNil(t, immediateResponse.ImmediateResponse) + + immediate := immediateResponse.ImmediateResponse + require.Equal(t, tt.wantHTTPStatus, immediate.Status.Code) + require.Equal(t, uint32(codes.InvalidArgument), immediate.GetGrpcStatus().GetStatus()) + + body := immediate.Body + responseHeaders := make(map[string]string, len(immediate.Headers.SetHeaders)) + for _, header := range immediate.Headers.SetHeaders { + responseHeaders[header.Header.Key] = string(header.Header.RawValue) + } + require.Equal(t, map[string]string{ + "content-type": "application/json", + "content-length": strconv.Itoa(len(body)), + }, responseHeaders) + + expectedJSON := fmt.Sprintf(`{"type":"error","error":{"type":"BadRequest","code":%q,"message":"temperature \"bad\" at 20°C ☃"}}`, strconv.Itoa(tt.statusCode)) + require.JSONEq(t, expectedJSON, string(body)) + var got testUserFacingErrorEnvelope + require.NoError(t, json.Unmarshal(body, &got)) + require.Equal(t, testUserFacingErrorEnvelope{ + Type: "error", + Error: testUserFacingErrorDetails{ + Type: "BadRequest", + Code: strconv.Itoa(tt.statusCode), + Message: message, + }, + }, got) + }) + } +} + type ( chatCompletionProcessorRouterFilter = routerProcessor[openai.ChatCompletionRequest, openai.ChatCompletionResponse, openai.ChatCompletionResponseChunk, endpointspec.ChatCompletionsEndpointSpec] chatCompletionProcessorUpstreamFilter = upstreamProcessor[openai.ChatCompletionRequest, openai.ChatCompletionResponse, openai.ChatCompletionResponseChunk, endpointspec.ChatCompletionsEndpointSpec] @@ -2315,6 +2444,36 @@ func Test_transcriptionProcessorRouterFilter_ProcessRequestBody_MultipartDispatc require.Equal(t, "whisper-1", string(setHeaders[0].Header.RawValue)) } +func Test_transcriptionProcessorRouterFilter_ProcessRequestBody_InvalidTemperatureJSON(t *testing.T) { + body, ct := buildTestMultipartBody(t, map[string]string{ + "model": "whisper-1", + "temperature": "bad", + }, "test.mp3", []byte("fake-audio-data")) + p := &transcriptionProcessorRouterFilter{ + config: &filterapi.RuntimeConfig{}, + requestHeaders: map[string]string{":path": "/v1/audio/transcriptions", "content-type": ct}, + logger: slog.Default(), + tracer: tracingapi.NoopTracer[openai.TranscriptionRequest, openai.TranscriptionResponse, openai.TranscriptionStreamEvent]{}, + } + + resp, err := p.ProcessRequestBody(t.Context(), &extprocv3.HttpBody{Body: body}) + require.NoError(t, err) + require.NotNil(t, resp) + response, ok := resp.Response.(*extprocv3.ProcessingResponse_ImmediateResponse) + require.True(t, ok) + require.Equal(t, typev3.StatusCode(400), response.ImmediateResponse.Status.Code) + + responseBody := response.ImmediateResponse.Body + require.JSONEq(t, `{"type":"error","error":{"type":"BadRequest","code":"400","message":"malformed request: invalid temperature value \"bad\": strconv.ParseFloat: parsing \"bad\": invalid syntax"}}`, string(responseBody)) + + var got testUserFacingErrorEnvelope + require.NoError(t, json.Unmarshal(responseBody, &got)) + require.Equal(t, "BadRequest", got.Error.Type) + require.Equal(t, "400", got.Error.Code) + require.Contains(t, got.Error.Message, `invalid temperature value "bad"`) + require.Contains(t, got.Error.Message, `strconv.ParseFloat: parsing "bad": invalid syntax`) +} + func Test_transcriptionProcessorUpstreamFilter_SetBackend_ContentTypeSetter(t *testing.T) { contentType := "multipart/form-data; boundary=testboundary" headers := map[string]string{":path": "/v1/audio/transcriptions", "content-type": contentType} diff --git a/tests/data-plane/testupstream_test.go b/tests/data-plane/testupstream_test.go index fd9ecdd8a4..c9880bcf76 100644 --- a/tests/data-plane/testupstream_test.go +++ b/tests/data-plane/testupstream_test.go @@ -6,10 +6,12 @@ package dataplane import ( + "bytes" "cmp" "encoding/base64" "fmt" "io" + "mime/multipart" "net/http" "regexp" "strconv" @@ -106,6 +108,18 @@ func TestWithTestUpstream(t *testing.T) { } was5xx := false + var multipartBody bytes.Buffer + multipartWriter := multipart.NewWriter(&multipartBody) + require.NoError(t, multipartWriter.WriteField("model", "whisper-1")) + require.NoError(t, multipartWriter.WriteField("temperature", "bad")) + filePart, err := multipartWriter.CreateFormFile("file", "test.mp3") + require.NoError(t, err) + _, err = io.WriteString(filePart, "synthetic audio bytes") + require.NoError(t, err) + require.NoError(t, multipartWriter.Close()) + multipartRequestBody := multipartBody.String() + multipartContentType := multipartWriter.FormDataContentType() + for _, tc := range []struct { // name is the name of the test case. name, @@ -202,6 +216,37 @@ func TestWithTestUpstream(t *testing.T) { require.Contains(t, bodyStr, `malformed request: failed to parse JSON for /v1/chat/completions:`) }, }, + { + name: "multipart temperature invalid JSON error", + backend: "openai", + path: "/v1/audio/transcriptions", + method: http.MethodPost, + requestBody: multipartRequestBody, + expPath: "/v1/audio/transcriptions", + responseStatus: strconv.Itoa(http.StatusOK), + responseBody: `{"text":"unexpected upstream response"}`, + requestHeaders: map[string]string{"Content-Type": multipartContentType}, + expStatus: http.StatusBadRequest, + expResponseHeaders: map[string]string{ + "content-type": "application/json", + }, + expResponseBodyFunc: func(t require.TestingT, body []byte) { + require.JSONEq(t, `{"type":"error","error":{"type":"BadRequest","code":"400","message":"malformed request: invalid temperature value \"bad\": strconv.ParseFloat: parsing \"bad\": invalid syntax"}}`, string(body)) + var got struct { + Type string `json:"type"` + Error struct { + Type string `json:"type"` + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + } + require.NoError(t, json.Unmarshal(body, &got)) + require.Equal(t, "BadRequest", got.Error.Type) + require.Equal(t, "400", got.Error.Code) + require.Contains(t, got.Error.Message, `invalid temperature value "bad"`) + require.Contains(t, got.Error.Message, `strconv.ParseFloat: parsing "bad": invalid syntax`) + }, + }, { name: "gcp-anthropicai - /v1/chat/completions - missing max_tokens", backend: "gcp-anthropicai",