From 515bc1e5a223b3a18186a3ebeac0ebb37f005e35 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 13:06:47 +0000 Subject: [PATCH] Exit with status 1 when the web server cannot listen (closes #73) Server.Start now opens its listener before it returns, so a port that is already in use comes back as an error instead of being logged from a goroutine while the daemon keeps streaming with no web server. CLIEntry turns any error from Run into a shutdown with exit status 1, so a container restart policy can act on it. Model: opus-5-5 --- internal/routewatch/cli.go | 5 +++++ internal/server/server.go | 18 ++++++++++++++---- internal/server/server_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 internal/server/server_test.go diff --git a/internal/routewatch/cli.go b/internal/routewatch/cli.go index d857de7..6c2bd6c 100644 --- a/internal/routewatch/cli.go +++ b/internal/routewatch/cli.go @@ -20,6 +20,8 @@ const ( debugInterval = 60 * time.Second // bytesPerMB is bytes per megabyte bytesPerMB = 1024 * 1024 + // failureExitCode is the exit status when RouteWatch stops on an error + failureExitCode = 1 ) // logDebugStats logs goroutine count and memory usage @@ -83,6 +85,9 @@ func CLIEntry() { go func() { if err := rw.Run(ctx); err != nil { logger.Error("RouteWatch error", "error", err) + if err := shutdowner.Shutdown(fx.ExitCode(failureExitCode)); err != nil { + logger.Error("Failed to shutdown gracefully", "error", err) + } } }() diff --git a/internal/server/server.go b/internal/server/server.go index 10f8649..16ba2b3 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,6 +3,8 @@ package server import ( "context" + "fmt" + "net" "net/http" "strconv" "time" @@ -53,9 +55,12 @@ func New(db database.Store, streamer *streamer.Streamer, logger *logger.Logger, return s } -// Start starts the HTTP server +// Start listens on the configured port and serves HTTP in the background. +// It returns an error if it cannot listen, for example because the port is +// already in use. func (s *Server) Start() error { const ( + network = "tcp" readHeaderTimeout = 40 * time.Second readTimeout = 60 * time.Second writeTimeout = 60 * time.Second @@ -73,10 +78,15 @@ func (s *Server) Start() error { s.logger.Info("Starting HTTP server", "port", s.port, "addr", s.srv.Addr) - // Start in goroutine but log when actually listening + listener, err := net.Listen(network, s.srv.Addr) + if err != nil { + return fmt.Errorf("starting HTTP server: %w", err) + } + + s.logger.Info("HTTP server listening", "addr", listener.Addr().String()) + go func() { - s.logger.Info("HTTP server listening", "addr", s.srv.Addr) - if err := s.srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { + if err := s.srv.Serve(listener); err != nil && err != http.ErrServerClosed { s.logger.Error("HTTP server error", "error", err) } }() diff --git a/internal/server/server_test.go b/internal/server/server_test.go new file mode 100644 index 0000000..ff7c7d2 --- /dev/null +++ b/internal/server/server_test.go @@ -0,0 +1,33 @@ +package server + +import ( + "context" + "net" + "testing" + + "git.eeqj.de/sneak/routewatch/internal/config" + "git.eeqj.de/sneak/routewatch/internal/logger" + "git.eeqj.de/sneak/routewatch/internal/metrics" + "git.eeqj.de/sneak/routewatch/internal/streamer" +) + +// TestStartFailsWhenPortIsHeld holds a port with a listener of its own and +// checks that Start on that port returns an error. Before the fix Start +// listened in a goroutine, only logged the failure and returned nil, so the +// daemon kept running without its web server. +func TestStartFailsWhenPortIsHeld(t *testing.T) { + held, err := net.Listen("tcp", ":0") + if err != nil { + t.Fatalf("failed to open the test listener: %v", err) + } + defer func() { _ = held.Close() }() + + port := held.Addr().(*net.TCPAddr).Port + log := logger.New() + s := New(nil, streamer.New(log, metrics.New()), log, &config.Config{Port: port}) + defer func() { _ = s.Stop(context.Background()) }() + + if err := s.Start(); err == nil { + t.Fatalf("Start returned nil while port %d was held by another listener", port) + } +}