diff --git a/enterprise/aibridgeproxyd/reload_test.go b/enterprise/aibridgeproxyd/reload_test.go index f07462d970c..ae83a00a07c 100644 --- a/enterprise/aibridgeproxyd/reload_test.go +++ b/enterprise/aibridgeproxyd/reload_test.go @@ -149,17 +149,15 @@ func newReloadTestHarness(t *testing.T) *reloadTestHarness { t.Helper() recorder := &aibridgedRecorder{} - bridged := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Keep-alives are disabled so the proxy cannot reuse a stale pooled + // connection to aibridged, which would surface as a bare EOF on + // Windows (see AIGOV-430). + bridged := testutil.NewUnstartedHTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { recorder.record(r.URL.Path) w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte("aibridged")) })) - // The proxy reuses pooled connections to aibridged, but net/http will - // not retry a POST on a closed pooled conn, so a stale reuse fails - // with a bare EOF on Windows. Force a fresh conn per request. - // https://github.com/coder/internal/issues/1564 (AIGOV-430) - bridged.Config.SetKeepAlivesEnabled(false) - t.Cleanup(bridged.Close) + bridged.Start() store := &providerStore{} metrics := aibridgeproxyd.NewMetrics(prometheus.NewRegistry()) diff --git a/testutil/http_server.go b/testutil/http_server.go new file mode 100644 index 00000000000..84d8036a130 --- /dev/null +++ b/testutil/http_server.go @@ -0,0 +1,23 @@ +package testutil + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// NewUnstartedHTTPServer is httptest.NewUnstartedServer with keep-alives +// disabled by default. Tests that proxy or pool connections to a bare +// httptest.Server can intermittently fail on Windows with a bare EOF when +// a stale pooled connection is reused, because net/http does not retry a +// non-replayable request (e.g. a POST) on a closed pooled connection. +// Disabling keep-alives forces a fresh connection per request, eliminating +// that class of flake. Call Start before sending requests. See +// https://github.com/coder/internal/issues/1564 (AIGOV-430). +func NewUnstartedHTTPServer(t testing.TB, handler http.Handler) *httptest.Server { + t.Helper() + srv := httptest.NewUnstartedServer(handler) + srv.Config.SetKeepAlivesEnabled(false) + t.Cleanup(srv.Close) + return srv +} diff --git a/testutil/http_server_test.go b/testutil/http_server_test.go new file mode 100644 index 00000000000..fb7fc2e2a72 --- /dev/null +++ b/testutil/http_server_test.go @@ -0,0 +1,50 @@ +package testutil_test + +import ( + "bufio" + "fmt" + "net" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/testutil" +) + +// TestNewUnstartedHTTPServerDisablesKeepAlives verifies the helper +// disables keep-alives on the underlying server, which is the property +// that prevents stale pooled-connection reuse (see AIGOV-430). A server +// with keep-alives disabled closes the connection after a response, so a +// second request on the same raw connection must fail. +func TestNewUnstartedHTTPServerDisablesKeepAlives(t *testing.T) { + t.Parallel() + + srv := testutil.NewUnstartedHTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + srv.Start() + + conn, err := net.Dial("tcp", srv.Listener.Addr().String()) + require.NoError(t, err, "dial") + defer conn.Close() + + send := func() error { + _, err := fmt.Fprintf(conn, "GET / HTTP/1.1\r\nHost: x\r\n\r\n") + if err != nil { + return err + } + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + if err != nil { + return err + } + _ = resp.Body.Close() + return nil + } + + // First request on this connection succeeds. + require.NoError(t, send(), "first request") + // Keep-alives disabled means the server closes the conn, so the + // second request on the same conn fails. + require.Error(t, send(), "second request on reused connection must fail") +}