feat: add configurable http.Server timeouts and statusWriter.Unwrap#110
Open
coolbit wants to merge 4 commits into
Open
feat: add configurable http.Server timeouts and statusWriter.Unwrap#110coolbit wants to merge 4 commits into
coolbit wants to merge 4 commits into
Conversation
statusWriter embedded http.ResponseWriter but exposed no Unwrap, so http.ResponseController could not reach the underlying net.Conn: per-request SetReadDeadline/SetWriteDeadline silently failed once WithHTTPStatus wrapped a handler. Add Unwrap so response-controller deadlines work through the wrapper. Flush is kept for existing SSE callers that type-assert http.Flusher directly.
A negative duration parses fine but would make every request time out immediately, so treat it like an invalid value (log and disable). Extract the env-to-Config wiring into serverTimeouts so the mapping is unit tested, guarding against a mismapped field. Add cases for negative and explicit-zero durations.
coolbit
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The
http.Serverbuilt byserver.newServerset none ofReadTimeout,ReadHeaderTimeout,WriteTimeoutorIdleTimeout. A slow or stalled client— e.g. one that sends request headers but never sends the body — holds a
handler goroutine until the upstream idle timeout (ALB, 60s) closes the
connection. We hit this in production on a page-server API.
Separately,
contexts.statusWriter(installed byWithHTTPStatus, part ofDefaultMiddleware) embeddedhttp.ResponseWriterwithout anUnwrap, sohttp.ResponseControllercould not reach the underlyingnet.Conn—per-request
SetReadDeadline/SetWriteDeadlinesilently failed. This blockedthe natural per-route fix.
What
server.Config: addReadTimeout,ReadHeaderTimeout,WriteTimeout,IdleTimeoutand map them onto thehttp.Server.service.ListenAndServe: read these fromSERVER_*env vars, log theeffective values at startup. Invalid or negative values are logged and
ignored (a negative deadline would make every request fail immediately).
contexts.statusWriter: addUnwrap()soResponseControllerdeadlineswork through the wrapper. Existing
Flush()is kept.Backward compatibility
Fully backward compatible — nothing changes for current users:
0⇒ disabled ⇒ identical totoday's behaviour. Only services that set
SERVER_*get timeouts.Unwrap()is purely additive. It does not change type assertions(
w.(http.Flusher)/w.(http.Hijacker)are unaffected), andFlush()isretained so existing SSE callers keep working.
How to use
Set on the service (e.g. ECS task definition):
SERVER_READ_HEADER_TIMEOUT=10s
SERVER_READ_TIMEOUT=30s
SERVER_WRITE_TIMEOUT=60s # must exceed the slowest legitimate handler
SERVER_IDLE_TIMEOUT=120s # keep larger than the A
Per-route overrides are now possible via
http.NewResponseController(w).SetReadDeadline(...)inside a route-scopedmiddleware.
Testing
go test ./server/ ./service/ ./contexts/— addedSERVER_*env parsing (incl. invalid/negative/zero), the env-to-field wiring,Unwrap,ResponseControllerreaching the connectand a Flusher regression guard.