Skip to content

GitLab

Unverified Commit 5b6a8e60 authored by Bruce MacDonald's avatar Bruce MacDonald Committed by GitHub
Browse files

api/client: handle non-json streaming errors (#13007) 

While processing the response stream during a chat or generation if an error is occurred it is parsed and returned to the user. The issue with the existing code is that this assumed the response would be valid JSON, which is not a safe assumption and caused cryptic error messages to be displayed due to parsing failures:
`invalid character 'i' looking for beginning of value`

This change updates the stream function to return the raw error string if it cant be parsed as JSON. This should help with debugging issues by making sure the actual error reaches the user.
parent 467bbc0d
Hide whitespace changes
Inline Side-by-side
Showing with 76 additions and 11 deletions
api/client.go
View file @ 5b6a8e60
...@@ -226,7 +226,14 @@ func (c *Client) stream(ctx context.Context, method, path string, data any, fn f ...@@ -226,7 +226,14 @@ func (c *Client) stream(ctx context.Context, method, path string, data any, fn f
bts := scanner.Bytes() bts := scanner.Bytes()
if err := json.Unmarshal(bts, &errorResponse); err != nil { if err := json.Unmarshal(bts, &errorResponse); err != nil {
return fmt.Errorf("unmarshal: %w", err) if response.StatusCode >= http.StatusBadRequest {
return StatusError{
StatusCode: response.StatusCode,
Status: response.Status,
ErrorMessage: string(bts),
}
}
return errors.New(string(bts))
} }
if response.StatusCode == http.StatusUnauthorized { if response.StatusCode == http.StatusUnauthorized {
......
api/client_test.go
View file @ 5b6a8e60
...@@ -55,6 +55,7 @@ func TestClientFromEnvironment(t *testing.T) { ...@@ -55,6 +55,7 @@ func TestClientFromEnvironment(t *testing.T) {
type testError struct { type testError struct {
message string message string
statusCode int statusCode int
raw bool // if true, write message as-is instead of JSON encoding
} }
func (e testError) Error() string { func (e testError) Error() string {
...@@ -111,6 +112,20 @@ func TestClientStream(t *testing.T) { ...@@ -111,6 +112,20 @@ func TestClientStream(t *testing.T) {
}, },
}, },
}, },
{
name: "plain text error response",
responses: []any{
"internal server error",
},
wantErr: "internal server error",
},
{
name: "HTML error page",
responses: []any{
"<html><body>404 Not Found</body></html>",
},
wantErr: "404 Not Found",
},
} }
for _, tc := range testCases { for _, tc := range testCases {
...@@ -135,6 +150,12 @@ func TestClientStream(t *testing.T) { ...@@ -135,6 +150,12 @@ func TestClientStream(t *testing.T) {
return return
} }
if str, ok := resp.(string); ok {
fmt.Fprintln(w, str)
flusher.Flush()
continue
}
if err := json.NewEncoder(w).Encode(resp); err != nil { if err := json.NewEncoder(w).Encode(resp); err != nil {
t.Fatalf("failed to encode response: %v", err) t.Fatalf("failed to encode response: %v", err)
} }
...@@ -173,9 +194,10 @@ func TestClientStream(t *testing.T) { ...@@ -173,9 +194,10 @@ func TestClientStream(t *testing.T) {
func TestClientDo(t *testing.T) { func TestClientDo(t *testing.T) {
testCases := []struct { testCases := []struct {
name string name string
response any response any
wantErr string wantErr string
wantStatusCode int
}{ }{
{ {
name: "immediate error response", name: "immediate error response",
...@@ -183,7 +205,8 @@ func TestClientDo(t *testing.T) { ...@@ -183,7 +205,8 @@ func TestClientDo(t *testing.T) {
message: "test error message", message: "test error message",
statusCode: http.StatusBadRequest, statusCode: http.StatusBadRequest,
}, },
wantErr: "test error message", wantErr: "test error message",
wantStatusCode: http.StatusBadRequest,
}, },
{ {
name: "server error response", name: "server error response",
...@@ -191,7 +214,8 @@ func TestClientDo(t *testing.T) { ...@@ -191,7 +214,8 @@ func TestClientDo(t *testing.T) {
message: "internal error", message: "internal error",
statusCode: http.StatusInternalServerError, statusCode: http.StatusInternalServerError,
}, },
wantErr: "internal error", wantErr: "internal error",
wantStatusCode: http.StatusInternalServerError,
}, },
{ {
name: "successful response", name: "successful response",
...@@ -203,6 +227,26 @@ func TestClientDo(t *testing.T) { ...@@ -203,6 +227,26 @@ func TestClientDo(t *testing.T) {
Success: true, Success: true,
}, },
}, },
{
name: "plain text error response",
response: testError{
message: "internal server error",
statusCode: http.StatusInternalServerError,
raw: true,
},
wantErr: "internal server error",
wantStatusCode: http.StatusInternalServerError,
},
{
name: "HTML error page",
response: testError{
message: "<html><body>404 Not Found</body></html>",
statusCode: http.StatusNotFound,
raw: true,
},
wantErr: "<html><body>404 Not Found</body></html>",
wantStatusCode: http.StatusNotFound,
},
} }
for _, tc := range testCases { for _, tc := range testCases {
...@@ -210,11 +254,16 @@ func TestClientDo(t *testing.T) { ...@@ -210,11 +254,16 @@ func TestClientDo(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if errResp, ok := tc.response.(testError); ok { if errResp, ok := tc.response.(testError); ok {
w.WriteHeader(errResp.statusCode) w.WriteHeader(errResp.statusCode)
err := json.NewEncoder(w).Encode(map[string]string{ if !errResp.raw {
"error": errResp.message, err := json.NewEncoder(w).Encode(map[string]string{
}) "error": errResp.message,
if err != nil { })
t.Fatal("failed to encode error response:", err) if err != nil {
t.Fatal("failed to encode error response:", err)
}
} else {
// Write raw message (simulates non-JSON error responses)
fmt.Fprint(w, errResp.message)
} }
return return
} }
...@@ -241,6 +290,15 @@ func TestClientDo(t *testing.T) { ...@@ -241,6 +290,15 @@ func TestClientDo(t *testing.T) {
if err.Error() != tc.wantErr { if err.Error() != tc.wantErr {
t.Errorf("error message mismatch: got %q, want %q", err.Error(), tc.wantErr) t.Errorf("error message mismatch: got %q, want %q", err.Error(), tc.wantErr)
} }
if tc.wantStatusCode != 0 {
if statusErr, ok := err.(StatusError); ok {
if statusErr.StatusCode != tc.wantStatusCode {
t.Errorf("status code mismatch: got %d, want %d", statusErr.StatusCode, tc.wantStatusCode)
}
} else {
t.Errorf("expected StatusError, got %T", err)
}
}
return return
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment