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 }