server: a port that cannot be bound stops startup (closes #266)
check / check (push) Waiting to run
check / check (push) Waiting to run
The start hook now binds the listen port itself and serves on it in a goroutine. A bind failure, such as a port already in use or a privileged port without root, is returned to fx, which stops startup and exits 1 with an error like "starting http server: listen tcp :8080: bind: address already in use". Before, the bind happened inside the goroutine and its error was only logged while the watcher kept running without the dashboard, API or health check. The http.Server is now built before that goroutine starts, so Shutdown no longer reads a field another goroutine may still be writing. TestPortInUseStopsStartup holds a port and checks that starting the server on it fails with an error naming it. Model: opus-5-5
This commit is contained in:
@@ -423,7 +423,7 @@ following precedence (highest to lowest):
|
|||||||
|
|
||||||
| Variable | Description | Default |
|
| 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_DEBUG` | Enable debug logging | `false` |
|
||||||
| `DNSWATCHER_DATA_DIR` | Directory for state file | `/var/lib/dnswatcher` |
|
| `DNSWATCHER_DATA_DIR` | Directory for state file | `/var/lib/dnswatcher` |
|
||||||
| `DNSWATCHER_TARGETS` | Comma-separated DNS names (auto-classified via PSL) | `""` |
|
| `DNSWATCHER_TARGETS` | Comma-separated DNS names (auto-classified via PSL) | `""` |
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
|
|||||||
|
|
||||||
# Completed Steps
|
# 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 bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false
|
- 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false
|
||||||
setting stops startup instead of being read as 0 or false (closes #269).
|
setting stops startup instead of being read as 0 or false (closes #269).
|
||||||
- 2026-10-13: a check saving the state while the dashboard or `/api/v1/status`
|
- 2026-10-13: a check saving the state while the dashboard or `/api/v1/status`
|
||||||
|
|||||||
@@ -13,8 +13,8 @@ import (
|
|||||||
const RequestTimeout time.Duration = requestTimeout
|
const RequestTimeout time.Duration = requestTimeout
|
||||||
|
|
||||||
// SetListenPort overrides the port Run binds. A test uses it to hand
|
// SetListenPort overrides the port Run binds. A test uses it to hand
|
||||||
// Run an unbindable port so ListenAndServe fails immediately and Run
|
// Run an unbindable port, so that Run stores its http.Server and then
|
||||||
// returns after storing its http.Server.
|
// returns an error without listening.
|
||||||
func SetListenPort(s *Server, port int) {
|
func SetListenPort(s *Server, port int) {
|
||||||
s.port = port
|
s.port = port
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -112,7 +113,7 @@ func New(
|
|||||||
}
|
}
|
||||||
|
|
||||||
lifecycle.Append(fx.Hook{
|
lifecycle.Append(fx.Hook{
|
||||||
OnStart: func(_ context.Context) error {
|
OnStart: func(ctx context.Context) error {
|
||||||
srv.startupTime = time.Now()
|
srv.startupTime = time.Now()
|
||||||
|
|
||||||
err := srv.enableSentry()
|
err := srv.enableSentry()
|
||||||
@@ -120,9 +121,7 @@ func New(
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
go srv.Run()
|
return srv.Run(ctx)
|
||||||
|
|
||||||
return nil
|
|
||||||
},
|
},
|
||||||
OnStop: func(ctx context.Context) error {
|
OnStop: func(ctx context.Context) error {
|
||||||
return srv.Shutdown(ctx)
|
return srv.Shutdown(ctx)
|
||||||
@@ -149,8 +148,10 @@ func newHTTPServer(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run starts the HTTP server.
|
// Run binds the listen port, then serves HTTP on it in a new goroutine
|
||||||
func (s *Server) Run() {
|
// 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()
|
s.SetupRoutes()
|
||||||
|
|
||||||
listenAddr := fmt.Sprintf(":%d", s.port)
|
listenAddr := fmt.Sprintf(":%d", s.port)
|
||||||
@@ -158,10 +159,21 @@ func (s *Server) Run() {
|
|||||||
|
|
||||||
s.log.Info("http server starting", "addr", listenAddr)
|
s.log.Info("http server starting", "addr", listenAddr)
|
||||||
|
|
||||||
err := s.httpServer.ListenAndServe()
|
lc := &net.ListenConfig{}
|
||||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
|
||||||
s.log.Error("http server error", "error", err)
|
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
|
// Shutdown gracefully shuts down the server, then sends the error
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package server_test
|
package server_test
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"net"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/spf13/viper"
|
"github.com/spf13/viper"
|
||||||
@@ -58,15 +60,13 @@ func buildServer(t *testing.T) *server.Server {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// TestRunWiresSocketTimeouts pins that the http.Server the running
|
// TestRunWiresSocketTimeouts pins that the http.Server the running
|
||||||
// server actually serves — the one Run builds and hands to
|
// server actually serves — the one Run builds and serves on the port
|
||||||
// ListenAndServe — carries every socket-level timeout, plus the two
|
// it binds — carries every socket-level timeout, plus the two
|
||||||
// relationships the values must satisfy.
|
// relationships the values must satisfy.
|
||||||
//
|
//
|
||||||
// Run is driven to completion with an unbindable port: it builds and
|
// Run is given an unbindable port: it builds and stores s.httpServer,
|
||||||
// stores s.httpServer, then ListenAndServe fails at once and Run
|
// then the bind fails and Run returns the error without ever
|
||||||
// returns without ever listening. The assertions run in the same
|
// listening. Nothing here measures elapsed time.
|
||||||
// goroutine after Run returns, so reading s.httpServer is free of any
|
|
||||||
// data race. Nothing here measures elapsed time.
|
|
||||||
//
|
//
|
||||||
// ReadTimeout must be at least ReadHeaderTimeout. net/http reads the
|
// ReadTimeout must be at least ReadHeaderTimeout. net/http reads the
|
||||||
// headers under ReadHeaderTimeout, then sets the read deadline for the
|
// headers under ReadHeaderTimeout, then sets the read deadline for the
|
||||||
@@ -84,7 +84,10 @@ func TestRunWiresSocketTimeouts(t *testing.T) {
|
|||||||
srv := buildServer(t)
|
srv := buildServer(t)
|
||||||
server.SetListenPort(srv, -1)
|
server.SetListenPort(srv, -1)
|
||||||
|
|
||||||
srv.Run()
|
err := srv.Run(t.Context())
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("Run bound port -1")
|
||||||
|
}
|
||||||
|
|
||||||
hs := server.HTTPServerOf(srv)
|
hs := server.HTTPServerOf(srv)
|
||||||
if hs == nil {
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user