Make request deadline extensions reach the socket behind the logging middleware
Build image / build-and-push (push) Successful in 6s
Gadfly review (reusable) / review (pull_request) Successful in 10m9s
Adversarial Review (Gadfly) / review (pull_request) Successful in 10m10s

Long agent turns were cut at exactly 30s on the live instance with "The
connection dropped partway through." — the #78 failure, which its tests
said was fixed. The tests host openEventStream on a bare gin.New(); in
production, slog-gin replaces c.Writer with a wrapper that embeds the
gin.ResponseWriter interface, which has no Unwrap, so the ResponseController
built from the handler's writer can't reach the connection and every
SetWriteDeadline returns ErrNotSupported. The stream fell back to the
server's absolute WriteTimeout; the first write past it failed, cancelled
the request context, and closed the socket under the client mid-frame.
The scan upload's read/write extensions failed the same way, with the
errors discarded.

captureController now runs first on the engine and stashes a controller
built before anything wraps the writer; openEventStream and scanSeedPacket
take it from responseController(c). The regression tests run the stream
through New() — the real stack, in the real order — and check from the
client side; the scan path logs once instead of swallowing the error.

Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
2026-08-22 22:56:21 -04:00
co-authored by Claude Fable 5
parent 5622b1accd
commit 2a903f6428
6 changed files with 151 additions and 15 deletions
+73 -10
View File
@@ -2,6 +2,7 @@ package api
import (
"bufio"
"net/http"
"net/http/httptest"
"strings"
"testing"
@@ -10,15 +11,22 @@ import (
"github.com/gin-gonic/gin"
)
// streamFrames spins up a real http.Server with the given WriteTimeout and an
// SSE handler that emits `frames` data frames, one every `tick`, then returns.
// It reports how many frames the client actually received and any read error —
// the only vantage point from which the deadline failures in #78/#87 are
// visible, since the writes themselves return nil when the bytes are dropped.
func streamFrames(t *testing.T, serverWriteTimeout, tick time.Duration, frames int) (int, error) {
t.Helper()
// bareEngine is a gin engine with NO middleware: the narrowest possible host for
// openEventStream, and what the #78/#87 tests were originally written against.
// It is not what production runs — the middleware stack in New wraps the
// ResponseWriter, and that difference is the whole subject of the third test.
func bareEngine() *gin.Engine {
gin.SetMode(gin.TestMode)
r := gin.New()
return gin.New()
}
// streamFrames spins up a real http.Server around r with the given WriteTimeout
// and an SSE route that emits `frames` data frames, one every `tick`, then
// returns. It reports how many frames the client actually received and any read
// error — the only vantage point from which the deadline failures in #78/#87 are
// visible, since the writes themselves return nil when the bytes are dropped.
func streamFrames(t *testing.T, r *gin.Engine, serverWriteTimeout, tick time.Duration, frames int) (int, error) {
t.Helper()
r.GET("/stream", func(c *gin.Context) {
s := openEventStream(c)
for i := 0; i < frames; i++ {
@@ -64,7 +72,7 @@ func TestEventStreamOutlivesServerWriteTimeout(t *testing.T) {
// keeps the stream alive with a huge margin — CI slowness only ever makes
// this pass more surely. The server's 300ms WriteTimeout is the thing being
// overridden; frames straddle it (300ms/600ms/900ms).
got, err := streamFrames(t, 300*time.Millisecond, 300*time.Millisecond, 3)
got, err := streamFrames(t, bareEngine(), 300*time.Millisecond, 300*time.Millisecond, 3)
if err != nil {
t.Errorf("client read error after %d/3 frames: %v", got, err)
}
@@ -90,7 +98,7 @@ func TestEventStreamRefreshesDeadlinePerFrame(t *testing.T) {
// The server WriteTimeout is generous (5s), so it isn't the limiter — the
// per-frame sseWriteTimeout is. 8 frames at a 100ms tick span 800ms, well past
// the 400ms deadline, but each 100ms gap is a 4× margin under it.
got, err := streamFrames(t, 5*time.Second, 100*time.Millisecond, 8)
got, err := streamFrames(t, bareEngine(), 5*time.Second, 100*time.Millisecond, 8)
if err != nil {
t.Errorf("client read error after %d/8 frames: %v", got, err)
}
@@ -99,3 +107,58 @@ func TestEventStreamRefreshesDeadlinePerFrame(t *testing.T) {
got, sseWriteTimeout)
}
}
// TestEventStreamOutlivesWriteTimeoutBehindMiddleware is #78 again, through the
// production middleware stack — which is where it was still broken.
//
// The two tests above passed while the deployed instance cut every agent turn
// at exactly 30s with "The connection dropped partway through." The difference
// is the middleware: the logging middleware in New replaces c.Writer with a
// wrapper that embeds the gin.ResponseWriter INTERFACE, which has no Unwrap. An
// http.ResponseController built from the handler's c.Writer unwraps layer by
// layer looking for SetWriteDeadline, can't see past that wrapper, and returns
// ErrNotSupported — putting the stream back on the server's absolute
// WriteTimeout. The first write past it fails, which cancels the request
// context and closes the socket under the client mid-frame.
//
// So: the same scenario as the first test, hosted on the engine New builds, in
// the order cmd/pansy runs it. Any future middleware that wraps the writer, or a
// reorder that puts one ahead of the controller capture, fails here.
func TestEventStreamOutlivesWriteTimeoutBehindMiddleware(t *testing.T) {
got, err := streamFrames(t, authEngine(t, localCfg()), 300*time.Millisecond, 300*time.Millisecond, 3)
if err != nil {
t.Errorf("client read error after %d/3 frames: %v", got, err)
}
if got != 3 {
t.Errorf("client received %d frames, want 3 — the stream was cut at the server WriteTimeout; the deadline override is not reaching the socket through the middleware stack", got)
}
}
// TestResponseControllerReachesTheSocketBehindMiddleware pins the mechanism the
// test above depends on, for every handler that extends a deadline — the scan
// upload extends both (seed_packet.go), and its calls were failing just as
// silently, with the errors discarded.
func TestResponseControllerReachesTheSocketBehindMiddleware(t *testing.T) {
r := authEngine(t, localCfg())
var readErr, writeErr error
r.GET("/deadlines", func(c *gin.Context) {
rc := responseController(c)
readErr = rc.SetReadDeadline(time.Now().Add(time.Minute))
writeErr = rc.SetWriteDeadline(time.Now().Add(time.Minute))
c.Status(http.StatusNoContent)
})
srv := httptest.NewServer(r)
defer srv.Close()
resp, err := srv.Client().Get(srv.URL + "/deadlines")
if err != nil {
t.Fatalf("get: %v", err)
}
resp.Body.Close()
if readErr != nil {
t.Errorf("SetReadDeadline through the production middleware: %v", readErr)
}
if writeErr != nil {
t.Errorf("SetWriteDeadline through the production middleware: %v", writeErr)
}
}