From c1616b31624092168ce1174438651e668d5cc65a Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sun, 23 Aug 2026 14:02:27 +0700 Subject: [PATCH] fix: series removal sent its status line twice adminSeriesRemove answered the list surface with two h.render calls -- the row fragment and the out-of-band heading -- and h.render writes a status line each time, so every removal logged "superfluous response.WriteHeader call". The heading is an append to a response already committed, so it now executes straight onto w, the way writeChromeOOB already does it. The regression test runs the router under a real server with a captured ErrorLog: a ResponseRecorder never sees this warning, which is why the existing removal test did not catch it. --- backend/internal/web/admin_series.go | 7 ++-- backend/web_test.go | 48 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/backend/internal/web/admin_series.go b/backend/internal/web/admin_series.go index 29f82a8..0b0d47c 100644 --- a/backend/internal/web/admin_series.go +++ b/backend/internal/web/admin_series.go @@ -505,10 +505,13 @@ func (h *Handler) adminSeriesRemove(w http.ResponseWriter, r *http.Request) { band = 1 } h.render(w, http.StatusOK, "series-row", seriesRow(a, band, time.Now())) + // The heading is an out-of-band append to a response whose status line has + // already gone out with the row, so it is executed straight onto w — + // h.render would send a second WriteHeader. if head, err := h.seriesListHeadView(r); err != nil { log.Printf("series remove %s: %v", site+":"+seriesID, err) - } else { - h.render(w, http.StatusOK, "series-list-head", head) + } else if err := h.tmpl.ExecuteTemplate(w, "series-list-head", head); err != nil { + log.Printf("series remove %s: render series-list-head oob: %v", site+":"+seriesID, err) } } diff --git a/backend/web_test.go b/backend/web_test.go index d5bc62c..827ac9e 100644 --- a/backend/web_test.go +++ b/backend/web_test.go @@ -1,11 +1,13 @@ package main import ( + "bytes" "crypto/sha256" "database/sql" "encoding/json" "fmt" "io" + "log" "net/http" "net/http/httptest" "net/url" @@ -4285,6 +4287,52 @@ func TestRemoveFromListAnswersRowAndFreshHeading(t *testing.T) { } } +// The removal answers two fragments on one response. The second is an +// out-of-band append, so it must not send a second status line: net/http +// answers a double WriteHeader with "superfluous response.WriteHeader call" +// on the server's error log, which a ResponseRecorder never sees. Hence a +// real server here. +func TestRemoveFromListSendsOneStatusLine(t *testing.T) { + st, dsn := newTestStoreURL(t) + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatalf("open %s: %v", dsn, err) + } + defer db.Close() + seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:a", url: "u", checkedAt: 9000, bookmarks: 0}) + + var errLog bytes.Buffer + srv := httptest.NewUnstartedServer(newRouter(st, testConfig())) + srv.Config.ErrorLog = log.New(&errLog, "", 0) + srv.Start() + defer srv.Close() + + req, err := http.NewRequest(http.MethodPost, srv.URL+"/admin/series/asura:a/remove", + strings.NewReader("filter=no_readers&band=0")) + if err != nil { + t.Fatalf("build request: %v", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(sessionCookie(t, st)) + resp, err := srv.Client().Do(req) + if err != nil { + t.Fatalf("removal request: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("removal status = %d, want 200", resp.StatusCode) + } + // Both fragments still travel: the fix must not have dropped the heading. + if !strings.Contains(string(body), "Title of asura:a") || + !strings.Contains(string(body), `hx-swap-oob="true"`) { + t.Errorf("answer lost a fragment:\n%s", body) + } + if strings.Contains(errLog.String(), "superfluous") { + t.Errorf("removal wrote the status line twice: %s", errLog.String()) + } +} + // A removal from the detail page navigates to the No-Readers list: htmx gets // a full navigation (HX-Redirect — a 303 would be followed by the request // and the list page swapped into the press's target), plain clients the 303