server: a port that cannot be bound stops startup (closes #266)

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

README.md

@@ -99,16 +99,11 @@ notification endpoint set, changes show only on the dashboard; see
   records, stored per nameserver. Their changes are notified as a hostname's
   are, as a record change, NS query failure, NS recovery, inconsistency or CNAME
   address change, in a message that starts `Domain:` where a hostname's starts
-  `Hostname:`. A nameserver removed from the domain's delegation gets only the
-  NS change notification above. A domain listed in
-  `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no record change or inconsistency
-  notification. A domain with no delegation of its own has these records asked
-  at the servers of the zone it is in, as a hostname has. A nameserver the
-  domain's records were asked at on the previous check but not on this one gets
-  a hostname's NS change notification (see below), unless it was in the domain's
-  delegation on the previous check, as when that delegation is withdrawn. A
-  domain that does not exist has none: they are not asked for, and those saved
-  by an earlier check are removed without a notification.
+  `Hostname:`. A domain listed in `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no
+  record change or inconsistency notification. A domain with no delegation of
+  its own has these records asked at the servers of the zone it is in, as a
+  hostname has. A domain that does not exist has none: they are not asked for,
+  and those saved by an earlier check are removed without a notification.
 
 ### DNS Hostname Monitoring (Subdomains)
 
@@ -161,10 +156,6 @@ notification endpoint set, changes show only on the dashboard; see
     - **NS recovery**: A previously-unreachable nameserver starts responding
       again. Its records are not compared with those from before it failed, so a
       change made while it was failing is not reported as a record change.
-    - **NS change**: A nameserver of the zone the hostname is in on the previous
-      check is not one now, whether or not it answered. One notification lists
-      every such nameserver on a `Removed:` line. Nothing is sent for a
-      nameserver that joins the zone.
     - **Inconsistency detected**: Two nameservers return different record sets
       for the same hostname and did not already differ on the previous check.
       Every pair of nameservers is compared. The alert is sent once for each
@@ -246,9 +237,7 @@ includes:
 
 - **DNS record changes**: Which hostname or domain, which nameserver, what
   record type, old values, new values.
-- **DNS NS changes**: Which domain, which nameservers were added to or removed
-  from its delegation. For a hostname, or a domain with no delegation of its
-  own, which nameservers left the zone it is in.
+- **DNS NS changes**: Which domain, which nameservers were added/removed.
 - **NS address changes**: Which domain, which nameserver, its old and new
   addresses.
 - **CNAME address changes**: Which hostname or domain, the old and new addresses
@@ -434,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)                                                                                   | `""`                  |
@@ -474,16 +463,6 @@ or empty variable (`DNSWATCHER_DNS_INTERVAL=`) means the default. If either is
 set to anything else, including a bare number or a zero or negative duration,
 dnswatcher refuses to start with an error naming the variable and the value.
 
-**`PORT` and `DNSWATCHER_TLS_EXPIRY_WARNING`** take a whole number: 1 to 65535
-for `PORT`, and a number of days, 1 or more, for
-`DNSWATCHER_TLS_EXPIRY_WARNING`. `DNSWATCHER_DEBUG`,
-`DNSWATCHER_MAINTENANCE_MODE` and `DNSWATCHER_SEND_TEST_NOTIFICATION` take
-`true` or `false`; `1`, `t`, `T`, `TRUE` and `True` also mean true, and `0`,
-`f`, `F`, `FALSE` and `False` also mean false. An unset or empty variable means
-the default. Any other value, such as `PORT=http`,
-`DNSWATCHER_TLS_EXPIRY_WARNING=7d` or `DNSWATCHER_DEBUG=yes`, makes dnswatcher
-refuse to start with an error naming the variable and the value.
-
 **`DNSWATCHER_SENTRY_DSN` reports crashes in HTTP requests to Sentry.** When it
 is set, a panic in an HTTP request handler is sent to Sentry, and the request
 still gets a `500 Internal Server Error` answer. Nothing else is sent to Sentry:

TODO.md

@@ -19,10 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 # Completed Steps
 
-- 2026-10-13: a nameserver leaving a domain's delegation sends only NS Change;
-  one leaving a hostname's zone sends NS Change, not NS Failure (closes #262).
-- 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).
+- 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

internal/config/config.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"log/slog"
 	"slices"
-	"strconv"
 	"strings"
 	"time"
 
@@ -25,9 +24,6 @@ const (
 	defaultTLSExpiryWarning = 7
 )
 
-// maxPort is the highest TCP port number.
-const maxPort = 65535
-
 // ErrNoTargets is returned when no monitoring targets are configured.
 var ErrNoTargets = errors.New(
 	"no monitoring targets configured: set DNSWATCHER_TARGETS environment variable",
@@ -44,23 +40,6 @@ var ErrInvalidInterval = errors.New(
 // lists a name that is not in DNSWATCHER_TARGETS.
 var ErrNotInTargets = errors.New("name is not in DNSWATCHER_TARGETS")
 
-// ErrInvalidPort is returned when PORT is set but is not a whole number
-// from 1 to 65535. An empty value counts as unset and means the default.
-var ErrInvalidPort = errors.New("port must be a whole number from 1 to 65535")
-
-// ErrInvalidTLSExpiryWarning is returned when DNSWATCHER_TLS_EXPIRY_WARNING
-// is set but is not a whole number, 1 or more. An empty value counts as unset
-// and means the default.
-var ErrInvalidTLSExpiryWarning = errors.New(
-	"TLS expiry warning must be a whole number of days, 1 or more",
-)
-
-// ErrInvalidBool is returned when DNSWATCHER_DEBUG,
-// DNSWATCHER_MAINTENANCE_MODE or DNSWATCHER_SEND_TEST_NOTIFICATION is set
-// but is not a value strconv.ParseBool accepts. An empty value counts as
-// unset and means the default.
-var ErrInvalidBool = errors.New("value must be true or false")
-
 // Params contains dependencies for Config.
 type Params struct {
 	fx.In
@@ -148,9 +127,16 @@ func buildConfig(
 	log *slog.Logger,
 	params *Params,
 ) (*Config, error) {
-	err := readConfigFile(log)
+	err := viper.ReadInConfig()
 	if err != nil {
-		return nil, err
+		var notFound viper.ConfigFileNotFoundError
+		if !errors.As(err, &notFound) {
+			log.Error("config file malformed", "error", err)
+
+			return nil, fmt.Errorf(
+				"config file malformed: %w", err,
+			)
+		}
 	}
 
 	dnsInterval, err := parseInterval("DNS_INTERVAL")
@@ -175,34 +161,9 @@ func buildConfig(
 		return nil, err
 	}
 
-	port, err := parsePort()
-	if err != nil {
-		return nil, err
-	}
-
-	tlsExpiryWarning, err := parseTLSExpiryWarning()
-	if err != nil {
-		return nil, err
-	}
-
-	debug, err := parseBool("DEBUG")
-	if err != nil {
-		return nil, err
-	}
-
-	maintenanceMode, err := parseBool("MAINTENANCE_MODE")
-	if err != nil {
-		return nil, err
-	}
-
-	sendTestNotification, err := parseBool("SEND_TEST_NOTIFICATION")
-	if err != nil {
-		return nil, err
-	}
-
 	cfg := &Config{
-		Port:                    port,
-		Debug:                   debug,
+		Port:                    viper.GetInt("PORT"),
+		Debug:                   viper.GetBool("DEBUG"),
 		DataDir:                 viper.GetString("DATA_DIR"),
 		Domains:                 domains,
 		Hostnames:               hostnames,
@@ -212,12 +173,12 @@ func buildConfig(
 		NtfyTopic:               viper.GetString("NTFY_TOPIC"),
 		DNSInterval:             dnsInterval,
 		TLSInterval:             tlsInterval,
-		TLSExpiryWarning:        tlsExpiryWarning,
+		TLSExpiryWarning:        viper.GetInt("TLS_EXPIRY_WARNING"),
 		SentryDSN:               viper.GetString("SENTRY_DSN"),
-		MaintenanceMode:         maintenanceMode,
+		MaintenanceMode:         viper.GetBool("MAINTENANCE_MODE"),
 		MetricsUsername:         viper.GetString("METRICS_USERNAME"),
 		MetricsPassword:         viper.GetString("METRICS_PASSWORD"),
-		SendTestNotification:    sendTestNotification,
+		SendTestNotification:    viper.GetBool("SEND_TEST_NOTIFICATION"),
 		params:                  params,
 		log:                     log,
 	}
@@ -225,21 +186,6 @@ func buildConfig(
 	return cfg, nil
 }
 
-// readConfigFile reads the config file, if there is one.
-func readConfigFile(log *slog.Logger) error {
-	err := viper.ReadInConfig()
-	if err != nil {
-		var notFound viper.ConfigFileNotFoundError
-		if !errors.As(err, &notFound) {
-			log.Error("config file malformed", "error", err)
-
-			return fmt.Errorf("config file malformed: %w", err)
-		}
-	}
-
-	return nil
-}
-
 // parseInterval reads the DNSWATCHER_-prefixed setting key as a duration. A
 // value that does not parse, or is zero or negative, is an error naming the
 // variable and the value; an unset variable has its default from setupViper.
@@ -256,53 +202,6 @@ func parseInterval(key string) (time.Duration, error) {
 	return interval, nil
 }
 
-// parsePort reads PORT, which has no DNSWATCHER_ prefix, as a port number. A
-// value that is not a whole number from 1 to 65535 is an error naming the
-// variable and the value; an unset variable has its default from setupViper.
-func parsePort() (int, error) {
-	value := viper.GetString("PORT")
-
-	port, err := strconv.Atoi(value)
-	if err != nil || port < 1 || port > maxPort {
-		return 0, fmt.Errorf("invalid PORT %q: %w", value, ErrInvalidPort)
-	}
-
-	return port, nil
-}
-
-// parseTLSExpiryWarning reads DNSWATCHER_TLS_EXPIRY_WARNING as a number of
-// days. A value that is not a whole number, 1 or more, is an error naming the
-// variable and the value; an unset variable has its default from setupViper.
-func parseTLSExpiryWarning() (int, error) {
-	value := viper.GetString("TLS_EXPIRY_WARNING")
-
-	days, err := strconv.Atoi(value)
-	if err != nil || days < 1 {
-		return 0, fmt.Errorf(
-			"invalid DNSWATCHER_TLS_EXPIRY_WARNING %q: %w",
-			value, ErrInvalidTLSExpiryWarning,
-		)
-	}
-
-	return days, nil
-}
-
-// parseBool reads the DNSWATCHER_-prefixed setting key as true or false. A
-// value strconv.ParseBool does not accept is an error naming the variable and
-// the value; an unset variable has its default from setupViper.
-func parseBool(key string) (bool, error) {
-	value := viper.GetString(key)
-
-	result, err := strconv.ParseBool(value)
-	if err != nil {
-		return false, fmt.Errorf(
-			"invalid DNSWATCHER_%s %q: %w", key, value, ErrInvalidBool,
-		)
-	}
-
-	return result, nil
-}
-
 func parseAndValidateTargets() ([]string, []string, error) {
 	domains, hostnames, err := ClassifyTargets(
 		parseCSV(viper.GetString("TARGETS")),

internal/config/config_test.go

@@ -116,7 +116,7 @@ func TestNew_OnlyEmptyCSVSegments(t *testing.T) {
 }
 
 // TestNew_InvalidIntervalStopsStartup checks values that must stop startup;
-// TestNew_DefaultValues and TestNew_EmptyValueMeansDefault check that an
+// TestNew_DefaultValues and TestNew_EmptyIntervalMeansDefault check that an
 // unset or empty interval means the default.
 func TestNew_InvalidIntervalStopsStartup(t *testing.T) {
 	variables := []string{"DNSWATCHER_DNS_INTERVAL", "DNSWATCHER_TLS_INTERVAL"}
@@ -144,107 +144,16 @@ func TestNew_InvalidIntervalStopsStartup(t *testing.T) {
 	}
 }
 
-func TestNew_EmptyValueMeansDefault(t *testing.T) {
+func TestNew_EmptyIntervalMeansDefault(t *testing.T) {
 	viper.Reset()
 	t.Setenv("DNSWATCHER_TARGETS", "example.com")
 	t.Setenv("DNSWATCHER_DNS_INTERVAL", "")
 	t.Setenv("DNSWATCHER_TLS_INTERVAL", "")
-	t.Setenv("PORT", "")
-	t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "")
-	t.Setenv("DNSWATCHER_DEBUG", "")
-	t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "")
-	t.Setenv("DNSWATCHER_SEND_TEST_NOTIFICATION", "")
 
 	cfg, err := config.New(nil, newTestParams(t))
 	require.NoError(t, err)
 	assert.Equal(t, time.Hour, cfg.DNSInterval)
 	assert.Equal(t, 12*time.Hour, cfg.TLSInterval)
-	assert.Equal(t, 8080, cfg.Port)
-	assert.Equal(t, 7, cfg.TLSExpiryWarning)
-	assert.False(t, cfg.Debug)
-	assert.False(t, cfg.MaintenanceMode)
-	assert.False(t, cfg.SendTestNotification)
-}
-
-// TestNew_InvalidNumberOrBoolStopsStartup checks values that must stop
-// startup; TestNew_EnvironmentOverrides, the two tests after this one and
-// TestNew_EmptyValueMeansDefault check values that must not.
-func TestNew_InvalidNumberOrBoolStopsStartup(t *testing.T) {
-	notBools := []string{"yes", "on", "2", "tRUE"}
-	tests := []struct {
-		variable string
-		values   []string
-		want     error
-	}{
-		{
-			variable: "PORT",
-			values:   []string{"notaport", "0", "-1", "65536", "8080.5"},
-			want:     config.ErrInvalidPort,
-		},
-		{
-			variable: "DNSWATCHER_TLS_EXPIRY_WARNING",
-			values:   []string{"abc", "7d", "0", "-5"},
-			want:     config.ErrInvalidTLSExpiryWarning,
-		},
-		{
-			variable: "DNSWATCHER_DEBUG",
-			values:   notBools,
-			want:     config.ErrInvalidBool,
-		},
-		{
-			variable: "DNSWATCHER_MAINTENANCE_MODE",
-			values:   notBools,
-			want:     config.ErrInvalidBool,
-		},
-		{
-			variable: "DNSWATCHER_SEND_TEST_NOTIFICATION",
-			values:   notBools,
-			want:     config.ErrInvalidBool,
-		},
-	}
-
-	for _, tt := range tests {
-		for _, value := range tt.values {
-			t.Run(tt.variable+"="+value, func(t *testing.T) {
-				viper.Reset()
-				t.Setenv("DNSWATCHER_TARGETS", "example.com")
-				t.Setenv(tt.variable, value)
-
-				_, err := config.New(nil, newTestParams(t))
-				require.ErrorIs(t, err, tt.want)
-				require.ErrorContains(t, err, tt.variable)
-				require.ErrorContains(t, err, strconv.Quote(value))
-			})
-		}
-	}
-}
-
-func TestNew_LowestNumbersAndOtherSpellingsOfTrue(t *testing.T) {
-	viper.Reset()
-	t.Setenv("DNSWATCHER_TARGETS", "example.com")
-	t.Setenv("PORT", "1")
-	t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "1")
-	t.Setenv("DNSWATCHER_DEBUG", "1")
-	t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "T")
-	t.Setenv("DNSWATCHER_SEND_TEST_NOTIFICATION", "True")
-
-	cfg, err := config.New(nil, newTestParams(t))
-	require.NoError(t, err)
-	assert.Equal(t, 1, cfg.Port)
-	assert.Equal(t, 1, cfg.TLSExpiryWarning)
-	assert.True(t, cfg.Debug)
-	assert.True(t, cfg.MaintenanceMode)
-	assert.True(t, cfg.SendTestNotification)
-}
-
-func TestNew_HighestPort(t *testing.T) {
-	viper.Reset()
-	t.Setenv("DNSWATCHER_TARGETS", "example.com")
-	t.Setenv("PORT", "65535")
-
-	cfg, err := config.New(nil, newTestParams(t))
-	require.NoError(t, err)
-	assert.Equal(t, 65535, cfg.Port)
 }
 
 func TestNew_DebugEnablesDebugLogging(t *testing.T) {

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
 }

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

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)
+	}
+}

internal/watcher/message_test.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 	"testing"
 
+	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/state"
 	"sneak.berlin/go/dnswatcher/internal/watcher"
 )
@@ -73,19 +74,21 @@ b.ns.example.net.: 192.0.2.2`,
 func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 	t.Parallel()
 
-	cfg := defaultTestConfig(t)
-	cfg.Domains = []string{domain}
-
-	deps := newTestDeps(t, cfg)
-	w := watcher.NewForTest(cfg, deps.state, nil, nil, nil, deps.notifier)
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(
+		&config.Config{Domains: []string{domain}},
+		nil, nil, nil, nil, notifier,
+	)
 
 	// nsA's address changes, which also makes it differ from nsC; nsB
-	// fails; nsC answers again.
+	// fails; nsC answers again; nsD is gone.
+	nsD := "d.ns.example.net."
 	w.DetectHostnameChanges(t.Context(), domain,
 		saved(map[string]*state.NameserverRecordState{
 			nsA: answered(map[string][]string{"A": {ip1}}),
 			nsB: answered(map[string][]string{"A": {ip1}}),
 			nsC: failed(),
+			nsD: answered(map[string][]string{"A": {ip1}}),
 		}),
 		saved(map[string]*state.NameserverRecordState{
 			nsA: answered(map[string][]string{"A": {ip2}}),
@@ -99,17 +102,18 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 		t.Context(), domain, cnameState(ip1), cnameState(ip2),
 	)
 
+	// NS Failure is sent for nsB failing and for nsD being gone.
 	want := map[string]int{
 		"Record Change":        1,
 		"Inconsistency":        1,
-		"NS Failure":           1,
+		"NS Failure":           2,
 		"NS Recovery":          1,
 		"CNAME Address Change": 1,
 	}
 
 	sent := make(map[string]int)
 
-	for _, n := range deps.notifier.getNotifications() {
+	for _, n := range notifier.getNotifications() {
 		kind, _, _ := strings.Cut(n.Title, ":")
 		sent[kind]++
 
@@ -124,89 +128,6 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 	}
 }
 
-// nsB, which answered, and nsC, which did not, are no longer among the
-// nameservers. One NS Change lists both, for the hostname and for the
-// configured apex domain with no delegation of its own. Nothing is sent
-// for the domain whose delegation on the previous check listed them:
-// the check of its delegation sends its NS Change.
-func TestRemovedNameserversAreAnNSChange(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name   string
-		target string
-		// The domain's nameservers saved by the previous check.
-		delegation []string
-		// The NS Change sent, or "" for none.
-		want string
-	}{
-		{
-			name:   "hostname",
-			target: host,
-			want: `Hostname: www.example.net
-Removed: b.ns.example.net., c.ns.example.net.`,
-		},
-		{
-			name:       "domain with no delegation of its own",
-			target:     domain,
-			delegation: []string{},
-			want: `Domain: example.net
-Removed: b.ns.example.net., c.ns.example.net.`,
-		},
-		{
-			name:       "domain whose delegation listed them",
-			target:     domain,
-			delegation: []string{nsA, nsB, nsC},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			cfg := defaultTestConfig(t)
-			cfg.Domains = []string{domain}
-
-			deps := newTestDeps(t, cfg)
-			w := watcher.NewForTest(
-				cfg, deps.state, nil, nil, nil, deps.notifier,
-			)
-
-			deps.state.SetDomainState(domain, &state.DomainState{
-				Nameservers: tt.delegation,
-			})
-
-			prev := saved(map[string]*state.NameserverRecordState{
-				nsA: answered(map[string][]string{"A": {ip1}}),
-				nsB: answered(map[string][]string{"A": {ip1}}),
-				nsC: failed(),
-			})
-			current := saved(map[string]*state.NameserverRecordState{
-				nsA: answered(map[string][]string{"A": {ip1}}),
-			})
-
-			w.DetectHostnameChanges(t.Context(), tt.target, prev, current)
-
-			got := deps.notifier.getNotifications()
-
-			if tt.want == "" {
-				if len(got) != 0 {
-					t.Errorf("sent %v, want nothing", got)
-				}
-
-				return
-			}
-
-			title := "NS Change: " + tt.target
-			if len(got) != 1 || got[0].Title != title ||
-				got[0].Message != tt.want {
-				t.Errorf("sent %v, want one %q with message:\n%s",
-					got, title, tt.want)
-			}
-		})
-	}
-}
-
 // The startup notification counts the configured domains and hostnames,
 // although the state's hostnames also hold the apex domain's own
 // records. Nothing is looked up: the watcher has no resolver.

internal/watcher/watcher.go

@@ -327,25 +327,25 @@ func (w *Watcher) checkDomain(
 		w.detectNSAddressChanges(ctx, domain, prevAddresses, addresses)
 	}
 
-	// A domain that does not exist has no records of its own: none are
-	// asked for, and those saved by an earlier check are removed.
-	// Otherwise the apex domain's records are also checked and saved as
-	// a hostname's, so that the port and TLS checks find its addresses.
-	// Notifications about them name it as a domain (see nameLine). They
-	// are checked before the domain's new nameservers are saved:
-	// detectNSDisappearances reads those of the previous check.
-	if nxdomain {
-		w.state.DeleteHostnameState(domain)
-	} else {
-		w.checkHostname(ctx, domain)
-	}
-
 	w.state.SetDomainState(domain, &state.DomainState{
 		Nameservers:         nameservers,
 		NameserverAddresses: addresses,
 		NXDomain:            nxdomain,
 		LastChecked:         now,
 	})
+
+	// A domain that does not exist has no records of its own: none are
+	// asked for, and those saved by an earlier check are removed.
+	if nxdomain {
+		w.state.DeleteHostnameState(domain)
+
+		return
+	}
+
+	// The apex domain's records are also checked and saved as a
+	// hostname's, so that the port and TLS checks find its addresses.
+	// Notifications about them name it as a domain (see nameLine).
+	w.checkHostname(ctx, domain)
 }
 
 func (w *Watcher) detectNSChanges(
@@ -772,52 +772,28 @@ func (w *Watcher) detectRecordChanges(
 	}
 }
 
-// detectNSDisappearances sends one NS Change listing the nameservers of
-// the previous check that are no longer those of the zone hostname is
-// in. For a configured apex domain it leaves out those of the domain's
-// delegation on the previous check, which detectNSChanges notifies when
-// they are removed from it; checkDomain saves the new delegation only
-// after this runs.
 func (w *Watcher) detectNSDisappearances(
 	ctx context.Context,
 	hostname string,
 	prev, current *state.HostnameState,
 ) {
-	var delegation []string
-
-	if w.isDomain(hostname) {
-		if domainState, ok := w.state.GetDomainState(hostname); ok {
-			delegation = domainState.Nameservers
+	for ns, prevNS := range prev.RecordsByNameserver {
+		if _, ok := current.RecordsByNameserver[ns]; ok || prevNS.Status != statusOK {
+			continue
 		}
+
+		msg := fmt.Sprintf(
+			"%s\nNameserver: %s disappeared",
+			w.nameLine(hostname), ns,
+		)
+
+		w.notify.SendNotification(
+			ctx,
+			"NS Failure: "+hostname,
+			msg,
+			"error",
+		)
 	}
-
-	var removed []string
-
-	for ns := range prev.RecordsByNameserver {
-		_, ok := current.RecordsByNameserver[ns]
-		if !ok && !slices.Contains(delegation, ns) {
-			removed = append(removed, ns)
-		}
-	}
-
-	if len(removed) == 0 {
-		return
-	}
-
-	slices.Sort(removed)
-
-	msg := fmt.Sprintf(
-		"%s\nRemoved: %s",
-		w.nameLine(hostname),
-		strings.Join(removed, ", "),
-	)
-
-	w.notify.SendNotification(
-		ctx,
-		"NS Change: "+hostname,
-		msg,
-		"warning",
-	)
 }
 
 // detectNSFailures notifies when a nameserver that answered on the

internal/watcher/watcher_test.go

@@ -558,9 +558,6 @@ func TestDomainThatDoesNotExist(t *testing.T) {
 // nameservers and without nxdomain, and its records, asked at the
 // codeberg.page servers, are saved. Those are testSmallDomain's two
 // nameservers; github.io, the zone of the README's example, has eight.
-// The previous check saved a delegation to oldNS1, now withdrawn, and
-// records asked at oldNS1: the NS Change of its delegation removes
-// oldNS1, and no other NS Change lists it again.
 func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	t.Parallel()
 
@@ -576,15 +573,6 @@ func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 
 		w, deps = newTestWatcher(t, cfg)
 
-		deps.state.SetDomainState(domain, &state.DomainState{
-			Nameservers: []string{oldNS1},
-		})
-		deps.state.SetHostnameState(domain, saved(
-			map[string]*state.NameserverRecordState{
-				oldNS1: answered(map[string][]string{"A": {oldIP}}),
-			},
-		))
-
 		err := checkOnce(ctx, w, deps)
 
 		// A domain saved as not existing has no records to wait for;
@@ -605,22 +593,6 @@ func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	if _, ok := deps.state.GetHostnameState(domain); !ok {
 		t.Errorf("no records saved for %s", domain)
 	}
-
-	title := "NS Change: " + domain
-	want := "Domain: " + domain + "\nAdded: \nRemoved: " + oldNS1
-
-	var changes []notification
-
-	for _, n := range deps.notifier.getNotifications() {
-		if n.Title == title {
-			changes = append(changes, n)
-		}
-	}
-
-	if len(changes) != 1 || changes[0].Message != want {
-		t.Errorf("sent %v, want one %q with message:\n%s",
-			changes, title, want)
-	}
 }
 
 func TestNSAddressChangeDetection(t *testing.T) {
@@ -1077,7 +1049,7 @@ func TestSendTestNotification_Disabled(t *testing.T) {
 	}
 }
 
-func TestNSChangeAndRecovery(t *testing.T) {
+func TestNSFailureAndRecovery(t *testing.T) {
 	t.Parallel()
 
 	cfg := defaultTestConfig(t)
@@ -1087,7 +1059,7 @@ func TestNSChangeAndRecovery(t *testing.T) {
 
 	// The saved state has every nameserver live DNS lists as one that
 	// did not answer, and, as answering, one that live DNS does not
-	// list, which the check then finds removed.
+	// list, which then disappears.
 	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		byNameserver := map[string]*state.NameserverRecordState{
 			oldNS1: answered(map[string][]string{"A": {oldIP}}),
@@ -1099,7 +1071,7 @@ func TestNSChangeAndRecovery(t *testing.T) {
 		deps.state.SetHostnameState(testHost, saved(byNameserver))
 	})
 
-	assertNotified(t, deps, "NS Change: "+testHost, "warning")
+	assertNotified(t, deps, "NS Failure: "+testHost, "error")
 	assertNotified(t, deps, "NS Recovery: "+testHost, "success")
 
 	// A nameserver that did not answer has no records to compare, so