From 0e306867d2ef46fb0483b05f565742f7074be8eb Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 20 Oct 2026 21:54:54 +0000 Subject: [PATCH] Fail startup when the HTTP listener cannot bind (closes #167) The HTTP server ran ListenAndServe in a goroutine, so a failed bind was only logged and neoircd kept running without its HTTP API. The start hook now binds the listen address itself and returns the bind error, so fx stops startup and neoircd exits 1 with an error naming the address. The bound listener is then served in a goroutine; if serving ends with anything other than a normal shutdown, the server stops the app through fx.Shutdowner with exit code 1. A new test holds a port and checks that starting the app on it fails with an error naming the address. Model: opus-5-5 --- internal/server/server.go | 50 ++++++++++++++++++-------- internal/server/server_test.go | 65 +++++++++++++++++++++++++++++----- 2 files changed, 93 insertions(+), 22 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index 49bcd50..0bd8951 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log/slog" + "net" "net/http" "os" "os/signal" @@ -39,6 +40,7 @@ type Params struct { Config *config.Config Middleware *middleware.Middleware Handlers *handlers.Handlers + Shutdowner fx.Shutdowner } // Server is the main HTTP server. @@ -66,7 +68,7 @@ func New( } lifecycle.Append(fx.Hook{ - OnStart: func(_ context.Context) error { + OnStart: func(ctx context.Context) error { srv.startupTime = time.Now() // Build the router before OnStart returns, so srv can @@ -78,9 +80,7 @@ func New( // A closed stdout or stderr must not end the process. signal.Ignore(syscall.SIGPIPE) - srv.startHTTP() - - return nil + return srv.startHTTP(ctx) }, OnStop: func(ctx context.Context) error { srv.cleanShutdown(ctx) @@ -162,15 +162,23 @@ func (srv *Server) configure() { // Server configuration placeholder. } -// startHTTP creates the HTTP server and serves it from a new -// goroutine. srv.httpServer is set before that goroutine -// starts and never changes, so it and cleanShutdown read it -// without a lock. -func (srv *Server) startHTTP() { +// startHTTP binds the HTTP listen address, so that a port +// that is taken fails startup, then serves the HTTP server +// from a new goroutine. srv.httpServer is set before that +// goroutine starts and never changes, so it and cleanShutdown +// read it without a lock. +func (srv *Server) startHTTP(ctx context.Context) error { listenAddr := fmt.Sprintf( ":%d", srv.params.Config.Port, ) + var listenConfig net.ListenConfig + + listener, err := listenConfig.Listen(ctx, "tcp", listenAddr) + if err != nil { + return fmt.Errorf("http listen: %w", err) + } + srv.httpServer = &http.Server{ //nolint:exhaustruct // optional fields Addr: listenAddr, ReadTimeout: httpReadTimeout, @@ -179,16 +187,30 @@ func (srv *Server) startHTTP() { Handler: srv, } - go srv.serveUntilShutdown() + go func() { + srv.serveUntilShutdown(listener) + }() + + return nil } -func (srv *Server) serveUntilShutdown() { +// serveUntilShutdown serves the HTTP server on listener until +// cleanShutdown stops it. If serving fails before then, it +// stops the app with exit code 1. +func (srv *Server) serveUntilShutdown(listener net.Listener) { srv.log.Info( "http begin listen", "listenaddr", srv.httpServer.Addr, ) - err := srv.httpServer.ListenAndServe() - if err != nil && !errors.Is(err, http.ErrServerClosed) { - srv.log.Error("listen error", "error", err) + err := srv.httpServer.Serve(listener) + if errors.Is(err, http.ErrServerClosed) { + return + } + + srv.log.Error("listen error", "error", err) + + err = srv.params.Shutdowner.Shutdown(fx.ExitCode(1)) + if err != nil { + srv.log.Error("shutdown failed", "error", err) } } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index c740ba1..71dfeb4 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "strconv" + "strings" "testing" "time" @@ -103,15 +104,68 @@ func TestStopWaitsForRequestInProgress(t *testing.T) { } } -// startApp starts the server with the dependencies -// cmd/neoircd gives it, on a fresh in-memory database, with -// the HTTP server listening on port. +// TestStartFailsWhenHTTPPortIsTaken checks that starting the +// app, which runs the start hook server.New registers, +// returns an error naming the HTTP listen address when +// something else is already listening on its port. +func TestStartFailsWhenHTTPPortIsTaken(t *testing.T) { + t.Parallel() + + var listenConfig net.ListenConfig + + taken, err := listenConfig.Listen(t.Context(), "tcp", ":0") + if err != nil { + t.Fatalf("listen: %v", err) + } + + t.Cleanup(func() { _ = taken.Close() }) + + tcpAddr, ok := taken.Addr().(*net.TCPAddr) + if !ok { + t.Fatalf("listener address %v is not TCP", taken.Addr()) + } + + app, _, _ := newApp(t, tcpAddr.Port) + + err = app.Start(t.Context()) + if err == nil { + app.RequireStop() + t.Fatal("the app started while its HTTP port was taken") + } + + listenAddr := fmt.Sprintf(":%d", tcpAddr.Port) + if !strings.Contains(err.Error(), listenAddr) { + t.Errorf("start error %q does not name %s", err, listenAddr) + } +} + +// startApp starts the app newApp builds, with the HTTP server +// listening on port. func startApp( t *testing.T, port int, ) (*fxtest.App, *broker.Broker, *db.Database) { t.Helper() + app, brk, database := newApp(t, port) + + app.RequireStart() + + // Stops the app if the test ends before stopping it. + t.Cleanup(app.RequireStop) + + return app, brk, database +} + +// newApp builds, without starting it, an app that runs the +// server with the dependencies cmd/neoircd gives it, on a +// fresh in-memory database, with the HTTP port set to port. +func newApp( + t *testing.T, + port int, +) (*fxtest.App, *broker.Broker, *db.Database) { + t.Helper() + var ( brk *broker.Broker database *db.Database @@ -148,11 +202,6 @@ func startApp( fx.Populate(&brk, &database), ) - app.RequireStart() - - // Stops the app if the test ends before stopping it. - t.Cleanup(app.RequireStop) - return app, brk, database }