Answer 504, not 408, when a request outlasts its time limit (closes #70)
check / check (push) Canceled after 0s

The hand-written time limit middleware answered 408 and wrote that
answer from its own goroutine while the handler could still be writing.
It is replaced by chi's middleware.Timeout, placed after
JSONResponseMiddleware so its 504 is not lost behind a 200.
JSONValidationMiddleware answers 504 with the JSON error body when the
time limit has passed and the handler wrote nothing. status.json and
/api/v1/stats answer 504 themselves when their 4-second database read
runs out, instead of 408 or an empty answer that became 500.

Model: opus-5-5
This commit is contained in:
2026-10-14 13:17:52 +00:00
parent 00e51d284f
commit 090c7c5ba0
4 changed files with 115 additions and 138 deletions
+94
View File
@@ -148,6 +148,100 @@ func TestStatsHandlersAnswerFromMemory(t *testing.T) {
}
}
// shortRequestDeadline is the deadline the tests below give a request. The
// server's 30-second limit and the stats handlers' 4-second limit inherit it,
// so the tests do not wait for either.
const shortRequestDeadline = 50 * time.Millisecond
// TestRequestPastTimeLimitAnswers504 sends a request through the server's
// router to a page and to a JSON route whose handlers wait for the time limit
// to pass and return without answering. The page answers 504; the JSON route
// answers 504 with the JSON error body. Before the fix both answered 408,
// written by the time limit middleware from its own goroutine while the
// handler could still be writing.
func TestRequestPastTimeLimitAnswers504(t *testing.T) {
s := New(nil, streamer.New(logger.New(), metrics.New()), logger.New(), &config.Config{})
waitForTimeLimit := func(_ http.ResponseWriter, r *http.Request) {
<-r.Context().Done()
}
// Registered the way setupRoutes registers pages and JSON routes.
s.router.Get("/slow-page", waitForTimeLimit)
s.router.Get("/slow.json", JSONValidationMiddleware(http.HandlerFunc(waitForTimeLimit)).ServeHTTP)
tests := []struct {
path string
wantJSONBody bool
}{
{path: "/slow-page"},
{path: "/slow.json", wantJSONBody: true},
}
for _, tt := range tests {
ctx, cancel := context.WithTimeout(context.Background(), shortRequestDeadline)
rec := httptest.NewRecorder()
s.router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, tt.path, nil).WithContext(ctx))
cancel()
if rec.Code != http.StatusGatewayTimeout {
t.Errorf("%s: status %d, want %d; body %s", tt.path, rec.Code, http.StatusGatewayTimeout, rec.Body)
}
if tt.wantJSONBody {
checkJSONErrorCode(t, tt.path, rec, http.StatusGatewayTimeout)
}
}
}
// TestStatsHandlersAnswer504WhenDatabaseReadRunsOut calls each stats handler
// with a store that does not answer before the handler's 4-second limit runs
// out, and checks that the handler itself answers 504 with the JSON error
// body. Before the fix status.json answered 408, and /api/v1/stats wrote
// nothing, which the JSON check on its route turned into 500.
func TestStatsHandlersAnswer504WhenDatabaseReadRunsOut(t *testing.T) {
release := make(chan struct{})
defer close(release)
db := blockingStatsDB{release: release}
s := New(db, streamer.New(logger.New(), metrics.New()), logger.New(), &config.Config{})
handlers := map[string]http.HandlerFunc{
"status.json": s.handleStatusJSON(),
"stats": s.handleStats(),
}
for name, handler := range handlers {
ctx, cancel := context.WithTimeout(context.Background(), shortRequestDeadline)
rec := httptest.NewRecorder()
handler(rec, httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx))
cancel()
if rec.Code != http.StatusGatewayTimeout {
t.Errorf("%s: status %d, want %d; body %s", name, rec.Code, http.StatusGatewayTimeout, rec.Body)
}
checkJSONErrorCode(t, name, rec, http.StatusGatewayTimeout)
}
}
// checkJSONErrorCode checks that rec holds the JSON error body with the given
// code.
func checkJSONErrorCode(t *testing.T, name string, rec *httptest.ResponseRecorder, want int) {
t.Helper()
var body struct {
Status string `json:"status"`
Error struct {
Code int `json:"code"`
} `json:"error"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
t.Errorf("%s: decoding the answer %q: %v", name, rec.Body, err)
return
}
if body.Status != "error" || body.Error.Code != want {
t.Errorf("%s: answer %s, want status \"error\" and error code %d", name, rec.Body, want)
}
}
// settledGoroutineCount lets transient goroutines finish, then reports the
// current count.
func settledGoroutineCount() int {