diff --git a/README.md b/README.md index 089ef95..185a837 100644 --- a/README.md +++ b/README.md @@ -423,7 +423,7 @@ following precedence (highest to lowest): | Variable | Description | Default | | -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | --------------------- | -| `PORT` | HTTP listen port | `8080` | +| `PORT` | HTTP listen port; a port that cannot be bound, such as one already in use, stops startup | `8080` | | `DNSWATCHER_DEBUG` | Enable debug logging | `false` | | `DNSWATCHER_DATA_DIR` | Directory for state file | `/var/lib/dnswatcher` | | `DNSWATCHER_TARGETS` | Comma-separated DNS names (auto-classified via PSL) | `""` | diff --git a/TODO.md b/TODO.md index 5c3c05e..9723474 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-13: a `PORT` that cannot be bound stops startup with an error naming + the address, instead of running without the HTTP server (closes #266). - 2026-10-13: a check saving the state while the dashboard or `/api/v1/status` is read cannot crash the process (closes #261). - 2026-10-06: canonical files re-fetched from `sneak/prompts` at `dd4027b`; lint diff --git a/internal/server/export_test.go b/internal/server/export_test.go index acd4d8c..0fecf2f 100644 --- a/internal/server/export_test.go +++ b/internal/server/export_test.go @@ -13,8 +13,8 @@ import ( const RequestTimeout time.Duration = requestTimeout // SetListenPort overrides the port Run binds. A test uses it to hand -// Run an unbindable port so ListenAndServe fails immediately and Run -// returns after storing its http.Server. +// Run an unbindable port, so that Run stores its http.Server and then +// returns an error without listening. func SetListenPort(s *Server, port int) { s.port = port } diff --git a/internal/server/server.go b/internal/server/server.go index e18a4d3..7f333bf 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log/slog" + "net" "net/http" "time" @@ -112,7 +113,7 @@ func New( } lifecycle.Append(fx.Hook{ - OnStart: func(_ context.Context) error { + OnStart: func(ctx context.Context) error { srv.startupTime = time.Now() err := srv.enableSentry() @@ -120,9 +121,7 @@ func New( return err } - go srv.Run() - - return nil + return srv.Run(ctx) }, OnStop: func(ctx context.Context) error { return srv.Shutdown(ctx) @@ -149,8 +148,10 @@ func newHTTPServer( } } -// Run starts the HTTP server. -func (s *Server) Run() { +// Run binds the listen port, then serves HTTP on it in a new goroutine +// and returns. A port that cannot be bound is returned as an error, so +// that startup stops instead of running without the HTTP server. +func (s *Server) Run(ctx context.Context) error { s.SetupRoutes() listenAddr := fmt.Sprintf(":%d", s.port) @@ -158,10 +159,21 @@ func (s *Server) Run() { s.log.Info("http server starting", "addr", listenAddr) - err := s.httpServer.ListenAndServe() - if err != nil && !errors.Is(err, http.ErrServerClosed) { - s.log.Error("http server error", "error", err) + lc := &net.ListenConfig{} + + listener, err := lc.Listen(ctx, "tcp", listenAddr) + if err != nil { + return fmt.Errorf("starting http server: %w", err) } + + go func() { + err := s.httpServer.Serve(listener) + if err != nil && !errors.Is(err, http.ErrServerClosed) { + s.log.Error("http server error", "error", err) + } + }() + + return nil } // Shutdown gracefully shuts down the server, then sends the error diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 8aa5f2d..297d505 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -1,6 +1,8 @@ package server_test import ( + "net" + "strings" "testing" "github.com/spf13/viper" @@ -58,15 +60,13 @@ func buildServer(t *testing.T) *server.Server { } // TestRunWiresSocketTimeouts pins that the http.Server the running -// server actually serves — the one Run builds and hands to -// ListenAndServe — carries every socket-level timeout, plus the two +// server actually serves — the one Run builds and serves on the port +// it binds — carries every socket-level timeout, plus the two // relationships the values must satisfy. // -// Run is driven to completion with an unbindable port: it builds and -// stores s.httpServer, then ListenAndServe fails at once and Run -// returns without ever listening. The assertions run in the same -// goroutine after Run returns, so reading s.httpServer is free of any -// data race. Nothing here measures elapsed time. +// Run is given an unbindable port: it builds and stores s.httpServer, +// then the bind fails and Run returns the error without ever +// listening. Nothing here measures elapsed time. // // ReadTimeout must be at least ReadHeaderTimeout. net/http reads the // headers under ReadHeaderTimeout, then sets the read deadline for the @@ -84,7 +84,10 @@ func TestRunWiresSocketTimeouts(t *testing.T) { srv := buildServer(t) server.SetListenPort(srv, -1) - srv.Run() + err := srv.Run(t.Context()) + if err == nil { + t.Fatal("Run bound port -1") + } hs := server.HTTPServerOf(srv) if hs == nil { @@ -133,3 +136,38 @@ func TestRunWiresSocketTimeouts(t *testing.T) { ) } } + +func TestPortInUseStopsStartup(t *testing.T) { + // Bound on every address, as Run binds, so that Run's bind fails. + lc := &net.ListenConfig{} + + taken, err := lc.Listen(t.Context(), "tcp", ":0") + if err != nil { + t.Fatalf("binding a port for the test: %v", err) + } + + defer func() { _ = taken.Close() }() + + _, port, err := net.SplitHostPort(taken.Addr().String()) + if err != nil { + t.Fatalf("reading the bound port: %v", err) + } + + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.com") + t.Setenv("DNSWATCHER_DATA_DIR", t.TempDir()) + t.Setenv("PORT", port) + + app := newServerApp(fx.Invoke(func(*server.Server) {})) + + err = app.Start(t.Context()) + if err == nil { + _ = app.Stop(t.Context()) + + t.Fatal("startup succeeded on a port that is already bound") + } + + if !strings.Contains(err.Error(), ":"+port) { + t.Errorf("startup error does not name port %s: %v", port, err) + } +}