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/handlers/api_test.go

@@ -325,7 +325,6 @@ 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,
@@ -333,7 +332,6 @@ 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,10 +1,6 @@
 package server
 
-import (
-	"net"
-
-	"github.com/go-chi/chi/v5"
-)
+import "github.com/go-chi/chi/v5"
 
 // NewTestSPAServer creates a Server whose router serves only
 // the embedded web client.
@@ -14,9 +10,3 @@ 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,7 +1,3 @@
-// Tests use a global viper instance for configuration,
-// making parallel execution unsafe.
-//
-//nolint:paralleltest
 package server_test
 
 import (
@@ -45,6 +41,8 @@ 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))
 
@@ -111,6 +109,8 @@ 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,63 +139,15 @@ 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, options...)
+	app, brk, database := newApp(t, port)
 
 	app.RequireStart()
 
@@ -208,12 +160,9 @@ 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()
 
@@ -251,7 +200,6 @@ func newApp(
 		}),
 		fx.Invoke(func(*server.Server) {}),
 		fx.Populate(&brk, &database),
-		fx.Options(options...),
 	)
 
 	return app, brk, database
@@ -262,21 +210,6 @@ 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(
@@ -286,12 +219,17 @@ func closedListener(t *testing.T) net.Listener {
 		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 listener
+	return tcpAddr.Port
 }
 
 // canConnect reports whether addr accepts a TCP connection.