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 and returns the bind
error, so neoircd exits 1 with an error naming the address. If serving
the bound listener later ends with anything other than a normal
shutdown, the server stops the app through fx.Shutdowner with exit
code 1; the handler tests now pass it that Shutdowner.

Tests check that a taken port fails startup, that a serve failure
stops the app with exit code 1, and that a normal stop does not. The
server tests no longer run in parallel: building the app writes the
global viper instance.

Model: opus-5-5

internal/handlers/api_test.go

@@ -325,6 +325,7 @@ func newTestServerFx(
 	cfg *config.Config,
 	mware *middleware.Middleware,
 	hdlr *handlers.Handlers,
+	shutdowner fx.Shutdowner,
 ) (*server.Server, error) {
 	srv, err := server.New(lifecycle, server.Params{ //nolint:exhaustruct
 		Logger:     log,
@@ -332,6 +333,7 @@ func newTestServerFx(
 		Config:     cfg,
 		Middleware: mware,
 		Handlers:   hdlr,
+		Shutdowner: shutdowner,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("test server: %w", err)

internal/server/export_test.go

@@ -1,6 +1,10 @@
 package server
 
-import "github.com/go-chi/chi/v5"
+import (
+	"net"
+
+	"github.com/go-chi/chi/v5"
+)
 
 // NewTestSPAServer creates a Server whose router serves only
 // the embedded web client.
@@ -10,3 +14,9 @@ func NewTestSPAServer() *Server {
 
 	return srv
 }
+
+// ServeUntilShutdown runs serveUntilShutdown, which serves the
+// HTTP server on listener, and returns when it does.
+func (srv *Server) ServeUntilShutdown(listener net.Listener) {
+	srv.serveUntilShutdown(listener)
+}

internal/server/server_test.go

@@ -1,3 +1,7 @@
+// Tests use a global viper instance for configuration,
+// making parallel execution unsafe.
+//
+//nolint:paralleltest
 package server_test
 
 import (
@@ -41,8 +45,6 @@ const (
 // for a long poll the HTTP server is serving to finish, and
 // that the poll gets its response.
 func TestStopWaitsForRequestInProgress(t *testing.T) {
-	t.Parallel()
-
 	port := freePort(t)
 	addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(port))
 
@@ -109,8 +111,6 @@ func TestStopWaitsForRequestInProgress(t *testing.T) {
 // 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")
@@ -139,15 +139,63 @@ func TestStartFailsWhenHTTPPortIsTaken(t *testing.T) {
 	}
 }
 
+// TestServeFailureStopsApp checks that when serving HTTP fails
+// after the start hook has bound the listen address, the
+// server stops the app with exit code 1.
+func TestServeFailureStopsApp(t *testing.T) {
+	var srv *server.Server
+
+	app, _, _ := startApp(t, freePort(t), fx.Populate(&srv))
+
+	shutdowns := app.Wait()
+
+	srv.ServeUntilShutdown(closedListener(t))
+
+	select {
+	case shutdown := <-shutdowns:
+		if shutdown.ExitCode != 1 {
+			t.Errorf("exit code %d, want 1", shutdown.ExitCode)
+		}
+	default:
+		t.Error("the server did not stop the app")
+	}
+}
+
+// TestServeAfterStopDoesNotStopApp checks that serving HTTP
+// after the app has stopped, which ends at once because the
+// stop hook has shut the HTTP server down, does not stop the
+// app. Serving ends the same way on every normal stop.
+func TestServeAfterStopDoesNotStopApp(t *testing.T) {
+	var srv *server.Server
+
+	app, _, _ := startApp(t, freePort(t), fx.Populate(&srv))
+
+	shutdowns := app.Wait()
+
+	app.RequireStop()
+
+	srv.ServeUntilShutdown(closedListener(t))
+
+	select {
+	case shutdown := <-shutdowns:
+		t.Errorf(
+			"the server stopped the app with exit code %d",
+			shutdown.ExitCode,
+		)
+	default:
+	}
+}
+
 // startApp starts the app newApp builds, with the HTTP server
 // listening on port.
 func startApp(
 	t *testing.T,
 	port int,
+	options ...fx.Option,
 ) (*fxtest.App, *broker.Broker, *db.Database) {
 	t.Helper()
 
-	app, brk, database := newApp(t, port)
+	app, brk, database := newApp(t, port, options...)
 
 	app.RequireStart()
 
@@ -160,9 +208,12 @@ func startApp(
 // 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.
+// options are added to the app, for a test that needs more
+// from it than the broker and the database.
 func newApp(
 	t *testing.T,
 	port int,
+	options ...fx.Option,
 ) (*fxtest.App, *broker.Broker, *db.Database) {
 	t.Helper()
 
@@ -200,6 +251,7 @@ func newApp(
 		}),
 		fx.Invoke(func(*server.Server) {}),
 		fx.Populate(&brk, &database),
+		fx.Options(options...),
 	)
 
 	return app, brk, database
@@ -210,6 +262,21 @@ func newApp(
 func freePort(t *testing.T) int {
 	t.Helper()
 
+	listener := closedListener(t)
+
+	tcpAddr, ok := listener.Addr().(*net.TCPAddr)
+	if !ok {
+		t.Fatalf("listener address %v is not TCP", listener.Addr())
+	}
+
+	return tcpAddr.Port
+}
+
+// closedListener returns a TCP listener that has already been
+// closed, so that accepting a connection on it fails.
+func closedListener(t *testing.T) net.Listener {
+	t.Helper()
+
 	var listenConfig net.ListenConfig
 
 	listener, err := listenConfig.Listen(
@@ -219,17 +286,12 @@ func freePort(t *testing.T) int {
 		t.Fatalf("listen: %v", err)
 	}
 
-	tcpAddr, ok := listener.Addr().(*net.TCPAddr)
-	if !ok {
-		t.Fatalf("listener address %v is not TCP", listener.Addr())
-	}
-
 	err = listener.Close()
 	if err != nil {
 		t.Fatalf("close listener: %v", err)
 	}
 
-	return tcpAddr.Port
+	return listener
 }
 
 // canConnect reports whether addr accepts a TCP connection.