From 72689c25528e63b4099d28c64630a29bf001745e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Mar 2026 21:41:24 +0200 Subject: [PATCH] fix(coderd): improve error handling in chattest, chattool, and chats (#23047) - Use t.Errorf in chattest non-streaming helpers so encoding failures fail the test - Thread testing.TB into writeResponsesAPIStreaming and log SSE write errors instead of silently dropping them - Bump createworkspace DB error log from Warn to Error - Use errors.Join for timeout + output error in execute.go --- coderd/chatd/chattest/anthropic.go | 2 +- coderd/chatd/chattest/errors.go | 2 +- coderd/chatd/chattest/openai.go | 14 ++++++++++---- coderd/chatd/chattool/createworkspace.go | 2 +- coderd/chatd/chattool/execute.go | 8 +++++--- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/coderd/chatd/chattest/anthropic.go b/coderd/chatd/chattest/anthropic.go index 22e7f8a66c..a93a655ba7 100644 --- a/coderd/chatd/chattest/anthropic.go +++ b/coderd/chatd/chattest/anthropic.go @@ -243,7 +243,7 @@ func (s *anthropicServer) writeNonStreamingResponse(w http.ResponseWriter, resp w.Header().Set("Content-Type", "application/json") w.Header().Set("anthropic-version", "2023-06-01") if err := json.NewEncoder(w).Encode(response); err != nil { - s.t.Logf("writeNonStreamingResponse: failed to encode response: %v", err) + s.t.Errorf("writeNonStreamingResponse: failed to encode response: %v", err) } } diff --git a/coderd/chatd/chattest/errors.go b/coderd/chatd/chattest/errors.go index 1091b7dff8..2c84339600 100644 --- a/coderd/chatd/chattest/errors.go +++ b/coderd/chatd/chattest/errors.go @@ -26,7 +26,7 @@ func writeErrorResponse(t testing.TB, w http.ResponseWriter, errResp *ErrorRespo }, } if err := json.NewEncoder(w).Encode(body); err != nil { - t.Logf("writeErrorResponse: failed to encode error response: %v", err) + t.Errorf("writeErrorResponse: failed to encode error response: %v", err) } } diff --git a/coderd/chatd/chattest/openai.go b/coderd/chatd/chattest/openai.go index 4c82182d19..6f19e08afe 100644 --- a/coderd/chatd/chattest/openai.go +++ b/coderd/chatd/chattest/openai.go @@ -228,7 +228,7 @@ func (s *openAIServer) writeResponsesAPIResponse(w http.ResponseWriter, req *Ope http.Error(w, "handler returned streaming response for non-streaming request", http.StatusInternalServerError) return case hasStreaming: - writeResponsesAPIStreaming(w, req.Request, resp.StreamingChunks) + writeResponsesAPIStreaming(s.t, w, req.Request, resp.StreamingChunks) default: s.writeResponsesAPINonStreaming(w, resp.Response) } @@ -320,7 +320,7 @@ func writeSSEEvent(w http.ResponseWriter, v interface{}) error { return err } -func writeResponsesAPIStreaming(w http.ResponseWriter, r *http.Request, chunks <-chan OpenAIChunk) { +func writeResponsesAPIStreaming(t testing.TB, w http.ResponseWriter, r *http.Request, chunks <-chan OpenAIChunk) { w.Header().Set("Content-Type", "text/event-stream") w.Header().Set("Cache-Control", "no-cache") w.Header().Set("Connection", "keep-alive") @@ -351,6 +351,7 @@ func writeResponsesAPIStreaming(w http.ResponseWriter, r *http.Request, chunks < ItemID: itemID, OutputIndex: int64(outputIndex), }); err != nil { + t.Logf("writeResponsesAPIStreaming: failed to write ResponseTextDoneEvent: %v", err) return } if err := writeSSEEvent(w, responses.ResponseOutputItemDoneEvent{ @@ -360,10 +361,12 @@ func writeResponsesAPIStreaming(w http.ResponseWriter, r *http.Request, chunks < Type: "message", }, }); err != nil { + t.Logf("writeResponsesAPIStreaming: failed to write ResponseOutputItemDoneEvent: %v", err) return } } if err := writeSSEEvent(w, responses.ResponseCompletedEvent{}); err != nil { + t.Logf("writeResponsesAPIStreaming: failed to write ResponseCompletedEvent: %v", err) return } flusher.Flush() @@ -390,6 +393,7 @@ func writeResponsesAPIStreaming(w http.ResponseWriter, r *http.Request, chunks < Type: "message", }, }); err != nil { + t.Logf("writeResponsesAPIStreaming: failed to write ResponseOutputItemAddedEvent: %v", err) return } flusher.Flush() @@ -407,10 +411,12 @@ func writeResponsesAPIStreaming(w http.ResponseWriter, r *http.Request, chunks < chunkBytes, err := json.Marshal(chunkData) if err != nil { + t.Logf("writeResponsesAPIStreaming: failed to marshal chunk data: %v", err) return } if _, err := fmt.Fprintf(w, "data: %s\n\n", chunkBytes); err != nil { + t.Logf("writeResponsesAPIStreaming: failed to write chunk data: %v", err) return } flusher.Flush() @@ -421,7 +427,7 @@ func writeResponsesAPIStreaming(w http.ResponseWriter, r *http.Request, chunks < func (s *openAIServer) writeChatCompletionsNonStreaming(w http.ResponseWriter, resp *OpenAICompletion) { w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(resp); err != nil { - s.t.Logf("writeChatCompletionsNonStreaming: failed to encode response: %v", err) + s.t.Errorf("writeChatCompletionsNonStreaming: failed to encode response: %v", err) } } @@ -452,7 +458,7 @@ func (s *openAIServer) writeResponsesAPINonStreaming(w http.ResponseWriter, resp } w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(response); err != nil { - s.t.Logf("writeResponsesAPINonStreaming: failed to encode response: %v", err) + s.t.Errorf("writeResponsesAPINonStreaming: failed to encode response: %v", err) } } diff --git a/coderd/chatd/chattool/createworkspace.go b/coderd/chatd/chattool/createworkspace.go index 70d3913e25..28eacbb27d 100644 --- a/coderd/chatd/chattool/createworkspace.go +++ b/coderd/chatd/chattool/createworkspace.go @@ -201,7 +201,7 @@ func CreateWorkspace(options CreateWorkspaceOptions) fantasy.AgentTool { Valid: true, }, }); err != nil { - options.Logger.Warn(ctx, "failed to persist chat workspace association", + options.Logger.Error(ctx, "failed to persist chat workspace association", slog.F("chat_id", options.ChatID), slog.F("workspace_id", workspace.ID), slog.Error(err), diff --git a/coderd/chatd/chattool/execute.go b/coderd/chatd/chattool/execute.go index a94266fd4f..6dec92eb08 100644 --- a/coderd/chatd/chattool/execute.go +++ b/coderd/chatd/chattool/execute.go @@ -3,12 +3,14 @@ package chattool import ( "context" "encoding/json" + "errors" "fmt" "regexp" "strings" "time" "charm.land/fantasy" + "golang.org/x/xerrors" "github.com/coder/coder/v2/codersdk/workspacesdk" ) @@ -248,15 +250,15 @@ func pollProcess( outputResp, outputErr := conn.ProcessOutput(bgCtx, processID) bgCancel() output := truncateOutput(outputResp.Output) - timeoutMsg := fmt.Sprintf("command timed out after %s", timeout) + timeoutErr := xerrors.Errorf("command timed out after %s", timeout) if outputErr != nil { - timeoutMsg += fmt.Sprintf(" (failed to get output: %v)", outputErr) + timeoutErr = errors.Join(timeoutErr, xerrors.Errorf("failed to get output: %w", outputErr)) } return ExecuteResult{ Success: false, Output: output, ExitCode: -1, - Error: timeoutMsg, + Error: timeoutErr.Error(), Truncated: outputResp.Truncated, } case <-ticker.C: