From 58b164b166281047d6169cccf94d21e524d407c8 Mon Sep 17 00:00:00 2001 From: Ashish Date: Sun, 2 Aug 2026 22:37:51 +0530 Subject: [PATCH] fix(errorpages): answer API clients with JSON instead of an HTML page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Traefik error-page middleware added in #52 is applied at the entrypoint level, so it wraps every router on both entrypoints — including the router for the Stackdome API server that the hub installer exposes as a StackResource. Any 5xx from the API therefore reached the dashboard as an HTML page, which axios cannot parse: the UI lost both the status and the message. Traefik copies the original request's headers onto the request it makes to the error-page service, so Accept still identifies the real caller. Serve the page to a browser navigation, and the API error envelope the dashboard already parses to everything else. The status code is unchanged either way. The match is on an explicit text/html rather than */*, because axios sends "application/json, text/plain, */*" — treating */* as a browser would reintroduce the bug for every API client. Note this cannot recover the backend's own error text: Traefik's errors middleware discards the caught response body before the error-page service is reached. Carrying the real message through needs Accept-split routers, which is a larger change. --- internal/errorpages/server.go | 26 ++++++++++++++++++--- internal/errorpages/server_test.go | 36 +++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/internal/errorpages/server.go b/internal/errorpages/server.go index e4ca533..2496999 100644 --- a/internal/errorpages/server.go +++ b/internal/errorpages/server.go @@ -27,18 +27,38 @@ func Handler() http.Handler { }) mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) { - status, page := pageFor(strings.TrimPrefix(req.URL.Path, "/")) - w.Header().Set("Content-Type", "text/html; charset=utf-8") + status, body := pageFor(strings.TrimPrefix(req.URL.Path, "/")) + contentType := "text/html; charset=utf-8" + if !wantsHTML(req) { + contentType, body = "application/json; charset=utf-8", errorJSON(status) + } + w.Header().Set("Content-Type", contentType) // These pages must never be cached: the same URL serves real content // again as soon as the workload recovers. w.Header().Set("Cache-Control", "no-store") w.WriteHeader(status) - _, _ = w.Write([]byte(page)) + _, _ = w.Write([]byte(body)) }) return mux } +// wantsHTML reports whether the caller is a browser navigation. Traefik's errors +// middleware copies the original request's headers onto the request it makes +// here, so Accept still describes the real client. The match is on an explicit +// text/html: axios sends "application/json, text/plain, */*", so matching */* +// too would hand every API client an HTML body it cannot parse. +func wantsHTML(req *http.Request) bool { + return strings.Contains(req.Header.Get("Accept"), "text/html") +} + +// errorJSON renders the failure as the API error envelope the dashboard already +// parses, so a 5xx Traefik swapped out reaches the UI in the shape a real API +// error would. StatusText is ASCII from a fixed table, so %q is enough quoting. +func errorJSON(status int) string { + return fmt.Sprintf(`{"type":"Error","reason":%q}`, http.StatusText(status)) +} + // pageFor resolves a request path to a status code and page body. An // unrecognised path is a catch-all 404; a status with no page of its own falls // back to the 500 page while keeping its real status code. diff --git a/internal/errorpages/server_test.go b/internal/errorpages/server_test.go index f530873..ab73ee3 100644 --- a/internal/errorpages/server_test.go +++ b/internal/errorpages/server_test.go @@ -15,9 +15,13 @@ var _ = Describe("Handler", func() { BeforeEach(func() { handler = Handler() }) + // The pages are what a browser gets, so the shared helper asks for HTML the + // way one does. The content-negotiation specs below set Accept themselves. get := func(path string) *httptest.ResponseRecorder { rec := httptest.NewRecorder() - handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil)) + req := httptest.NewRequest(http.MethodGet, path, nil) + req.Header.Set("Accept", "text/html") + handler.ServeHTTP(rec, req) return rec } @@ -56,6 +60,36 @@ var _ = Describe("Handler", func() { Expect(get("/some/deep/path").Body.String()).To(Equal(Assets["404.html"])) }) + // The middleware wraps every router on both entrypoints, so the dashboard's + // own /api/v1 calls reach these pages too. Traefik copies the caller's + // headers onto the request it makes here, so Accept picks the body shape. + DescribeTable("picks the body shape from the caller's Accept header", + func(accept, wantType, wantBody string) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/500", nil) + if accept != "" { + req.Header.Set("Accept", accept) + } + Handler().ServeHTTP(rec, req) + + Expect(rec.Code).To(Equal(http.StatusInternalServerError)) + Expect(rec.Header().Get("Content-Type")).To(ContainSubstring(wantType)) + Expect(rec.Body.String()).To(Equal(wantBody)) + }, + Entry("a browser navigation gets the page", + "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", + "text/html", Assets["500.html"]), + // The */* here is why the match is on an explicit text/html: treating + // */* as "browser" would hand axios an HTML body it cannot parse. + Entry("axios gets the API error envelope", + "application/json, text/plain, */*", + "application/json", `{"type":"Error","reason":"Internal Server Error"}`), + // Browsers always name text/html when navigating, so no Accept at all + // is a plain HTTP client. + Entry("a client sending no Accept gets the envelope", + "", "application/json", `{"type":"Error","reason":"Internal Server Error"}`), + ) + It("answers its own health check without a page body", func() { rec := get("/healthz")