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

@@ -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 srv.startHTTP(ctx)
-
-			return nil
 		},
 		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
+// startHTTP binds the HTTP listen address, so that a port
-// goroutine. srv.httpServer is set before that goroutine
+// that is taken fails startup, then serves the HTTP server
-// starts and never changes, so it and cleanShutdown read it
+// from a new goroutine. srv.httpServer is set before that
-// without a lock.
+// goroutine starts and never changes, so it and cleanShutdown
-func (srv *Server) startHTTP() {
+// 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()
+	err := srv.httpServer.Serve(listener)
-	if err != nil && !errors.Is(err, http.ErrServerClosed) {
+	if errors.Is(err, http.ErrServerClosed) {
-		srv.log.Error("listen error", "error", err)
+		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)
 	}
 }

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
+// TestStartFailsWhenHTTPPortIsTaken checks that starting the
-// cmd/neoircd gives it, on a fresh in-memory database, with
+// app, which runs the start hook server.New registers,
-// the HTTP server listening on port.
+// 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
 }