internal/server/http.go

@@ -43,10 +43,7 @@ func (s *Server) serveUntilShutdown() {
 	err := s.httpServer.ListenAndServe()
 	if err != nil && !errors.Is(err, http.ErrServerClosed) {
 		s.log.Error("listen error", "error", err)
-
-		if s.cancelFunc != nil {
-			s.cancelFunc()
-		}
+		s.shutdownOnListenFailure()
 	}
 }
 

internal/server/listen_failure_test.go

@@ -0,0 +1,95 @@
+package server_test
+
+import (
+	"context"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/server"
+)
+
+// listenFailureDeadline is how long the app gets to give up after a
+// listen it cannot satisfy. The defect this pins left the process
+// reporting RUNNING for 183 seconds with nothing bound; a bind error
+// is known instantly, so anything past a moment here is that defect
+// back.
+const listenFailureDeadline = 2 * time.Second
+
+// lifecycleTimeout bounds the app's start and stop sequences so a
+// wedged hook fails the test instead of hanging it.
+const lifecycleTimeout = 15 * time.Second
+
+// TestListenFailure_ShutsDownTheApp pins that a listener the server
+// cannot bind terminates the application with a non-zero status.
+//
+// The fx OnStart hook returns as soon as the serving goroutine is
+// spawned, so a bind failure is discovered after fx has already
+// reported RUNNING. Nothing else in the graph observes it, and the
+// process used to stay alive with no listener: down, but indis-
+// tinguishable from healthy to systemd's Restart=on-failure and to
+// Docker's restart policies, which is the state this test exists to
+// keep from returning.
+//
+// The port is occupied by a listener this test holds open, on a
+// kernel-chosen port, so the failure is the real EADDRINUSE the
+// operator hits when a second instance starts. Loopback is enough to
+// collide with the server's wildcard bind: a listening socket on a
+// specific address blocks the wildcard from claiming the same port.
+func TestListenFailure_ShutsDownTheApp(t *testing.T) {
+	t.Parallel()
+
+	var listenCfg net.ListenConfig
+
+	occupied, err := listenCfg.Listen(
+		t.Context(), "tcp", "127.0.0.1:0",
+	)
+	require.NoError(t, err)
+
+	t.Cleanup(func() { _ = occupied.Close() })
+
+	addr, ok := occupied.Addr().(*net.TCPAddr)
+	require.True(t, ok, "listener is not TCP")
+
+	// The collaborators come from the wired graph rather than stubs,
+	// so the Server under test is the one that ships. Only the port
+	// is test-specific.
+	env := newTestEnv(t)
+	env.cfg.Port = addr.Port
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Supply(env.log, env.cfg, env.mw, env.hnd),
+		fx.Provide(globals.New, server.New),
+		fx.Invoke(func(*server.Server) {}),
+	)
+
+	startCtx, cancelStart := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStart()
+
+	require.NoError(t, app.Start(startCtx))
+
+	select {
+	case sig := <-app.Wait():
+		require.Equal(
+			t, server.ListenFailureExitCode, sig.ExitCode,
+			"listen failure must exit non-zero",
+		)
+	case <-time.After(listenFailureDeadline):
+		t.Fatal("listen failure left the app running")
+	}
+
+	// The stop sequence still has to complete: the fix must reach
+	// shutdown through fx rather than around it.
+	stopCtx, cancelStop := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStop()
+
+	require.NoError(t, app.Stop(stopCtx))
+}

internal/server/server.go

@@ -50,6 +50,13 @@ const (
 	minSentryFlush = 250 * time.Millisecond
 )
 
+// ListenFailureExitCode is the status the process exits with when the
+// HTTP listener cannot be established, or dies for a reason other
+// than a requested shutdown. It must stay non-zero: systemd
+// `Restart=on-failure` and Docker's restart policies key off it, and a
+// zero exit would read as a deliberate stop.
+const ListenFailureExitCode = 1
+
 // SentryFlushBudget reports how long the Sentry flush may run when
 // remaining is the time left on the fx stop context after the HTTP
 // drain. sentry.Flush takes a bare duration and honours no context,
@@ -75,13 +82,13 @@ type ServerParams struct {
 	Config     *config.Config
 	Middleware *middleware.Middleware
 	Handlers   *handlers.Handlers
+	Shutdowner fx.Shutdowner
 }
 
 // Server is the main HTTP server that wires up routes and manages
 // graceful shutdown.
 type Server struct {
 	startupTime   time.Time
-	exitCode      int
 	sentryEnabled bool
 	log           *slog.Logger
 	cancelFunc    context.CancelFunc
@@ -159,7 +166,12 @@ func (s *Server) enableSentry() {
 	s.sentryEnabled = true
 }
 
-func (s *Server) serve() int {
+// serve installs the signal watcher, starts the listener and blocks
+// until the server's context is cancelled. The process exit status is
+// fx's to decide — from a signal, or from the code
+// shutdownOnListenFailure hands the Shutdowner — so this reports
+// nothing back to its caller.
+func (s *Server) serve() {
 	ctx, cancelFunc := context.WithCancel(context.Background())
 	s.cancelFunc = cancelFunc
 
@@ -185,7 +197,30 @@ func (s *Server) serve() int {
 	<-ctx.Done()
 	// Shutdown is handled by the fx OnStop hook (cleanShutdown).
 	// Do not call cleanShutdown() here to avoid double invocation.
-	return s.exitCode
+}
+
+// shutdownOnListenFailure ends the application after the HTTP
+// listener failed. The fx OnStart hook returns as soon as the serving
+// goroutine is spawned, so nothing downstream of it ever learns that
+// the listen failed: fx reports RUNNING and the process sits alive
+// with nothing bound, which is invisible to systemd and Docker
+// restart policies. Asking the Shutdowner to stop the app with a
+// non-zero code is what turns that into a visible failure.
+//
+// The context cancel that follows only unwinds serve()'s own wait.
+// The shutdown itself runs through fx's normal stop sequence, so the
+// clean-shutdown drain in cleanShutdown is reached unchanged.
+func (s *Server) shutdownOnListenFailure() {
+	err := s.params.Shutdowner.Shutdown(
+		fx.ExitCode(ListenFailureExitCode),
+	)
+	if err != nil {
+		s.log.Error("shutdown request failed", "error", err)
+	}
+
+	if s.cancelFunc != nil {
+		s.cancelFunc()
+	}
 }
 
 func (s *Server) cleanupForExit() {
@@ -193,9 +228,6 @@ func (s *Server) cleanupForExit() {
 }
 
 func (s *Server) cleanShutdown(ctx context.Context) {
-	// initiate clean shutdown
-	s.exitCode = 0
-
 	ctxShutdown, shutdownCancel := context.WithTimeout(
 		ctx, ShutdownTimeout,
 	)