fix: wait for HTTP server drain during graceful shutdown - #1745

pkg/server/server_test.go (1)

43-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider testing actual in-flight request drain behavior.

The test verifies ServeConnect returns after context cancellation, but doesn't verify the core PR claim: that in-flight requests finish draining before ServeConnect returns. A long-running handler that blocks until a signal would prove the drain actually waits. Without this, a regression that removes shutdownWG.Wait() would still pass this test (since Shutdown closes the listener quickly).

💡 Suggested drain-verification test case

func TestServeConnectDrainsInflightRequests(t *testing.T) {
	logger := slog.New(slog.NewTextHandler(io.Discard, nil))

	var cfg Config
	cfg.Connect.Port = freePort(t)

	// Register a slow handler via a custom mux that ServeConnect can use.
	// Since ServeConnect builds its own mux, you'd need to either:
	// 1. Add a test-only hook to inject handlers, or
	// 2. Use the /ping endpoint with a deliberate delay via a wrapper.
	//
	// Minimal approach: verify that after cancel, an in-flight /ping
	// request still completes successfully.

	// This would require either a test hook in ServeConnect or
	// a separate test that directly exercises server.Shutdown + WaitGroup.
}

This may require a small test hook in ServeConnect to inject a slow handler; if that's too invasive, consider a unit test for the shutdown coordination logic in isolation.

pkg/server/server.go (1)

207-210: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Set ReadHeaderTimeout to mitigate Slowloris attacks.

Both HTTP servers are instantiated without a read timeout. Without a timeout, a slow or malicious client can hold connections open indefinitely, potentially exhausting server resources. Setting ReadHeaderTimeout is a standard best practice to bound how long the server waits while reading request headers.

• pkg/server/server.go#L207-L210: add ReadHeaderTimeout: 5 * time.Second to the connect server configuration.
• pkg/server/server.go#L232-L235: add ReadHeaderTimeout: 5 * time.Second to the metrics server configuration.

Source: Linters/SAST tools