From 2feb6ea5a3bd77d2d329e930dfab9b14be7109b4 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Wed, 10 Jun 2026 16:22:35 +0200 Subject: [PATCH] api: keep the error contract JSON even on encoder failure writeJSON's last-resort branch (json.Marshal failing, a programmer error: unsupported type or cyclic data) previously fell back to http.Error, which answers text/plain and was the one corner where the API broke its own {"error": ...} contract. It now writes a hand-written constant JSON literal: still strictly simpler than the encoder that just failed (the reason writeError, which is built ON writeJSON, cannot be used there: it would be circular), but contract consistent. Tested by forcing the branch with a channel payload, which json.Marshal cannot encode. --- internal/api/api.go | 7 ++++++- internal/api/api_test.go | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/internal/api/api.go b/internal/api/api.go index 080bd64..38dc419 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -109,7 +109,12 @@ func handleScenario(w http.ResponseWriter, r *http.Request) { func writeJSON(w http.ResponseWriter, status int, payload any) { body, err := json.Marshal(payload) if err != nil { - http.Error(w, "encoding response failed", http.StatusInternalServerError) + // Last-resort path: report the encoder's failure without using the + // encoder. The hand-written constant keeps the error contract JSON + // while staying strictly simpler than what just failed. + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"encoding response failed"}`)) return } w.Header().Set("Content-Type", "application/json") diff --git a/internal/api/api_test.go b/internal/api/api_test.go index e235aaf..2af740b 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -22,6 +22,29 @@ func serve(t *testing.T, method, path string, body []byte) *httptest.ResponseRec return recorder } +func TestWriteJSONEncodingFailureStaysJSON(t *testing.T) { + // Channels cannot be marshaled, forcing the otherwise unreachable + // last-resort branch. Even there the error contract must stay JSON. + recorder := httptest.NewRecorder() + writeJSON(recorder, http.StatusOK, make(chan int)) + + if recorder.Code != http.StatusInternalServerError { + t.Errorf("status = %d, want %d", recorder.Code, http.StatusInternalServerError) + } + if got := recorder.Header().Get("Content-Type"); got != "application/json" { + t.Errorf("Content-Type = %q, want application/json", got) + } + var response struct { + Error string `json:"error"` + } + if err := json.Unmarshal(recorder.Body.Bytes(), &response); err != nil { + t.Fatalf("fallback body is not JSON: %v (body: %s)", err, recorder.Body) + } + if response.Error == "" { + t.Error("fallback body has an empty error field") + } +} + func TestDefaultConfigEndpoint(t *testing.T) { recorder := serve(t, http.MethodGet, "/api/config/default", nil)