watcher: notify NS query failure and recovery (closes #104)

LookupAllRecords now returns each nameserver's response, so the
watcher saves its status: ok when it answered, NXDOMAIN and no records
included, and error with the reason when it timed out, answered
SERVFAIL or REFUSED, or could not be reached. A nameserver that starts
failing sends NS Failure and one that answers again sends NS Recovery.
A failing nameserver is left out of the record change and
inconsistency comparisons. The resolver used to report REFUSED and
network errors as an answer with no records; they are now errors. A
lookup cut short by its context now returns an error instead of a
failure of the nameserver it was querying.

Model: opus-5-5

README.md

@@ -71,18 +71,25 @@ rejected.
     did on the previous check (additions, removals, value changes).
   - **NS query failure**: A nameserver that previously responded
     becomes unreachable (timeout, SERVFAIL, REFUSED, network error).
-    This is distinct from "responded with no records."
+    This is distinct from "responded with no records": a nameserver
+    that answers NXDOMAIN or with no records has responded. The alert
+    is sent once, on the check where it starts failing. A failing
+    nameserver gives no records, so it is not reported as a record
+    change or compared for inconsistency. A nameserver that is already
+    failing on the first check that sees it is recorded silently.
   - **NS recovery**: A previously-unreachable nameserver starts
-    responding again.
+    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.
   - **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 such pair, on the check where they start to disagree, and not
     again while they keep disagreeing, including after a restart. A
     nameserver that was not in the previous check (newly added, or back
-    after dropping out) and answers differently is reported on the check
+    after dropping out), or failed on it, and answers differently is
-    where it appears. If a pair agrees again and later disagrees, the
+    reported on the check where it answers. If a pair agrees again and
-    alert is sent again.
+    later disagrees, the alert is sent again.
 
 ### TCP Port Monitoring
 
@@ -442,9 +449,13 @@ The `status` field for each per-nameserver entry and certificate entry
 tracks reachability:
 
 | Status      | Meaning                                                    |
-|-------------|-------------------------------------------------|
+|-------------|------------------------------------------------------------|
 | `ok`        | Query succeeded, records are current                       |
-| `error`     | Query failed (timeout, SERVFAIL, network error)  |
+| `error`     | Query failed (timeout, SERVFAIL, REFUSED, network error)   |
+
+A nameserver that answers NXDOMAIN or with no records has status `ok` and
+empty `records`. A nameserver whose query failed has status `error`, empty
+`records`, and the reason in `error`.
 
 ---
 

TODO.md

@@ -19,6 +19,8 @@ Rationale, Design, TODO, License, Author) if any are still missing.
 
 # Completed Steps
 
+- 2026-10-01: a nameserver that does not answer is saved as `error` with the
+  reason, and NS failure and NS recovery are notified (closes #104).
 - 2026-09-29: the live-DNS test package is renamed `internal/livednstest` and
   added to the `test-support` `deny` list in `.golangci.yml`, so `make lint`
   fails when program code imports it (closes #164).

internal/resolver/iterative.go

@@ -504,7 +504,9 @@ func (r *Resolver) queryAllTypes(
 type queryState struct {
 	gotNXDomain bool
 	gotSERVFAIL bool
+	gotRefused  bool
 	gotTimeout  bool
+	netErr      error
 	hasRecords  bool
 }
 
@@ -542,8 +544,13 @@ func (r *Resolver) querySingleType(
 ) {
 	msg, err := r.queryDNS(ctx, nsIP, hostname, qtype)
 	if err != nil {
-		if isTimeout(err) {
+		switch {
+		case isTimeout(err):
 			state.gotTimeout = true
+		case errors.Is(err, ErrRefused):
+			state.gotRefused = true
+		default:
+			state.netErr = err
 		}
 
 		return
@@ -603,6 +610,12 @@ func classifyResponse(resp *NameserverResponse, state queryState) {
 	case state.gotSERVFAIL && !state.hasRecords:
 		resp.Status = StatusError
 		resp.Error = "server returned SERVFAIL"
+	case state.gotRefused && !state.hasRecords:
+		resp.Status = StatusError
+		resp.Error = "server returned REFUSED"
+	case state.netErr != nil && !state.hasRecords:
+		resp.Status = StatusError
+		resp.Error = "network error: " + state.netErr.Error()
 	case !state.hasRecords && !state.gotNXDomain:
 		resp.Status = StatusNoData
 	}
@@ -682,11 +695,14 @@ func (r *Resolver) queryEachNS(
 	results := make(map[string]*NameserverResponse)
 
 	for _, ns := range nameservers {
+		resp, err := r.QueryNameserver(ctx, ns, hostname)
+
+		// A query the context cut short says nothing about the
+		// nameserver, so it must not be returned as its failure.
 		if checkCtx(ctx) != nil {
 			return nil, ErrContextCanceled
 		}
 
-		resp, err := r.QueryNameserver(ctx, ns, hostname)
 		if err != nil {
 			results[ns] = &NameserverResponse{
 				Nameserver: ns,
@@ -714,21 +730,13 @@ func (r *Resolver) LookupNS(
 
 // LookupAllRecords performs iterative resolution to find all DNS
 // records for the given hostname, keyed by authoritative nameserver.
+// Each nameserver's response carries its status and error with its
+// records.
 func (r *Resolver) LookupAllRecords(
 	ctx context.Context,
 	hostname string,
-) (map[string]map[string][]string, error) {
+) (map[string]*NameserverResponse, error) {
-	results, err := r.QueryAllNameservers(ctx, hostname)
+	return r.QueryAllNameservers(ctx, hostname)
-	if err != nil {
-		return nil, err
-	}
-
-	out := make(map[string]map[string][]string, len(results))
-	for ns, resp := range results {
-		out[ns] = resp.Records
-	}
-
-	return out, nil
 }
 
 // ResolveIPAddresses resolves a hostname to all IPv4 and IPv6

internal/resolver/resolver_test.go

@@ -2,6 +2,7 @@ package resolver_test
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"net"
 	"os"
@@ -13,6 +14,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"sneak.berlin/go/dnswatcher/internal/livednstest"
 	"sneak.berlin/go/dnswatcher/internal/resolver"
 )
 
@@ -231,6 +233,45 @@ func TestQueryNameserver_NXDomain(t *testing.T) {
 	assert.Equal(t, resolver.StatusNXDomain, resp.Status)
 }
 
+// TestQueryNameserver_Refused asks a google.com nameserver about
+// cloudflare.com, a zone it does not serve, which it refuses. Refusing
+// is a failure to answer, not an answer with no records.
+func TestQueryNameserver_Refused(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	ns := findOneNSForDomain(t, r, "google.com")
+
+	var resp *resolver.NameserverResponse
+
+	livednstest.Retry(
+		t,
+		"QueryNameserver("+ns+", cloudflare.com)",
+		func(ctx context.Context) error {
+			var err error
+
+			resp, err = r.QueryNameserver(ctx, ns, "cloudflare.com")
+			if err != nil {
+				return err
+			}
+
+			// A timeout or a network error is no reply at all.
+			if resp.Status == resolver.StatusTimeout ||
+				strings.HasPrefix(resp.Error, "network error") {
+				return fmt.Errorf(
+					"%w: %s: %s",
+					livednstest.ErrNoAnswer, ns, resp.Error,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	assert.Equal(t, resolver.StatusError, resp.Status)
+	assert.Equal(t, "server returned REFUSED", resp.Error)
+}
+
 func TestQueryNameserver_RecordsSorted(t *testing.T) {
 	t.Parallel()
 
@@ -530,7 +571,7 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
 	// Nothing answers at 192.0.2.1, a documentation address. The
 	// resolver tries each query twice, and the first try gives up
 	// after two seconds. A deadline that ends during the first try
-	// makes the status vary from run to run between nodata and
+	// makes the status vary from run to run between error and
 	// timeout, so the deadline must outlast the first try.
 	ctx, cancel := context.WithTimeout(
 		context.Background(), 3*time.Second,

internal/watcher/export_test.go

@@ -2,14 +2,15 @@ package watcher
 
 import (
 	"context"
+	"time"
 
+	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/state"
 )
 
 // NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing.
 func NewlyDisagreeingPairs(
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) [][2]string {
 	return newlyDisagreeingPairs(prev, current)
 }
@@ -18,8 +19,15 @@ func NewlyDisagreeingPairs(
 func (w *Watcher) DetectHostnameChanges(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) {
 	w.detectHostnameChanges(ctx, hostname, prev, current)
 }
+
+// BuildHostnameState exports buildHostnameState for testing.
+func BuildHostnameState(
+	results map[string]*resolver.NameserverResponse,
+	now time.Time,
+) *state.HostnameState {
+	return buildHostnameState(results, now)
+}

internal/watcher/inconsistency_test.go

@@ -105,7 +105,9 @@ func TestNewlyDisagreeingPairs(t *testing.T) {
 
 			prev := hostnameState(tt.loaded)
 
-			for i, current := range tt.checks {
+			for i, records := range tt.checks {
+				current := hostnameState(records)
+
 				got := watcher.NewlyDisagreeingPairs(prev, current)
 				if !slices.Equal(got, tt.want[i]) {
 					t.Errorf(
@@ -114,7 +116,7 @@ func TestNewlyDisagreeingPairs(t *testing.T) {
 					)
 				}
 
-				prev = hostnameState(current)
+				prev = current
 			}
 		})
 	}
@@ -162,8 +164,9 @@ func TestInconsistencyAlert(t *testing.T) {
 			prev := hostnameState(tt.loaded)
 
 			for range 3 {
-				w.DetectHostnameChanges(t.Context(), host, prev, disagree)
+				current := hostnameState(disagree)
-				prev = hostnameState(disagree)
+				w.DetectHostnameChanges(t.Context(), host, prev, current)
+				prev = current
 			}
 
 			got := 0

internal/watcher/interfaces.go

@@ -5,6 +5,7 @@ import (
 	"context"
 
 	"sneak.berlin/go/dnswatcher/internal/portcheck"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/tlscheck"
 )
 
@@ -17,11 +18,11 @@ type DNSResolver interface {
 	) ([]string, error)
 
 	// LookupAllRecords queries all record types for a hostname,
-	// returning results keyed by nameserver then record type.
+	// returning each nameserver's response keyed by nameserver.
 	LookupAllRecords(
 		ctx context.Context,
 		hostname string,
-	) (map[string]map[string][]string, error)
+	) (map[string]*resolver.NameserverResponse, error)
 
 	// ResolveIPAddresses resolves a hostname to all IP addresses.
 	ResolveIPAddresses(

internal/watcher/nsfailure_test.go

@@ -0,0 +1,209 @@
+package watcher_test
+
+import (
+	"context"
+	"log/slog"
+	"strings"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/dnswatcher/internal/resolver"
+	"sneak.berlin/go/dnswatcher/internal/state"
+	"sneak.berlin/go/dnswatcher/internal/watcher"
+)
+
+// answered is what a check saves for a nameserver that answered with
+// these records.
+func answered(records map[string][]string) *state.NameserverRecordState {
+	return &state.NameserverRecordState{Records: records, Status: "ok"}
+}
+
+// failed is what a check saves for a nameserver that did not answer.
+func failed() *state.NameserverRecordState {
+	return &state.NameserverRecordState{
+		Records: map[string][]string{},
+		Status:  "error",
+		Error:   "all queries timed out",
+	}
+}
+
+// saved builds the hostname state a check saves.
+func saved(
+	byNameserver map[string]*state.NameserverRecordState,
+) *state.HostnameState {
+	return &state.HostnameState{RecordsByNameserver: byNameserver}
+}
+
+// alertCounts counts the hostname alerts sent, by kind.
+type alertCounts struct {
+	failures, recoveries, recordChanges, inconsistencies int
+}
+
+// countAlerts runs the hostname change detection from the state loaded
+// at startup through each check in turn, and counts the alerts sent.
+func countAlerts(
+	t *testing.T,
+	loaded *state.HostnameState,
+	checks []*state.HostnameState,
+) alertCounts {
+	t.Helper()
+
+	// The hostname change detection uses only the notifier.
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
+
+	prev := loaded
+
+	for _, current := range checks {
+		w.DetectHostnameChanges(t.Context(), host, prev, current)
+		prev = current
+	}
+
+	var got alertCounts
+
+	for _, n := range notifier.getNotifications() {
+		kind, _, _ := strings.Cut(n.Title, ":")
+
+		switch kind {
+		case "NS Failure":
+			got.failures++
+		case "NS Recovery":
+			got.recoveries++
+		case "Record Change":
+			got.recordChanges++
+		case "Inconsistency":
+			got.inconsistencies++
+		}
+	}
+
+	return got
+}
+
+func TestNSFailureAndRecoveryAlerts(t *testing.T) {
+	t.Parallel()
+
+	records := map[string][]string{"A": {ip1}}
+
+	bothAnswer := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(records),
+	})
+	bFails := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: failed(),
+	})
+	onlyA := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records),
+	})
+	bAnswersNoRecords := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(map[string][]string{}),
+	})
+	bAnswersDifferently := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(map[string][]string{"A": {ip2}}),
+	})
+
+	// Each case starts from the state loaded at startup and runs the
+	// checks in order.
+	tests := []struct {
+		name   string
+		loaded *state.HostnameState
+		checks []*state.HostnameState
+		want   alertCounts
+	}{
+		{
+			"failure lasting several checks alerts once",
+			bothAnswer, []*state.HostnameState{bFails, bFails, bFails},
+			alertCounts{failures: 1},
+		},
+		{
+			"recovery alerts once",
+			bFails, []*state.HostnameState{bothAnswer, bothAnswer},
+			alertCounts{recoveries: 1},
+		},
+		{
+			"failing again after recovering alerts again",
+			bothAnswer, []*state.HostnameState{bFails, bothAnswer, bFails},
+			alertCounts{failures: 2, recoveries: 1},
+		},
+		{
+			"nameserver failing when first seen does not alert",
+			onlyA, []*state.HostnameState{bFails, bFails},
+			alertCounts{},
+		},
+		{
+			"answer with no records is a record change, not a failure",
+			bothAnswer, []*state.HostnameState{bAnswersNoRecords},
+			alertCounts{recordChanges: 1, inconsistencies: 1},
+		},
+		{
+			"recovered nameserver that answers differently disagrees",
+			bFails, []*state.HostnameState{bAnswersDifferently},
+			alertCounts{recoveries: 1, inconsistencies: 1},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := countAlerts(t, tt.loaded, tt.checks)
+			if got != tt.want {
+				t.Errorf("sent %+v, want %+v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestNSFailureAlertNamesNameserverAndReason(t *testing.T) {
+	t.Parallel()
+
+	records := map[string][]string{"A": {ip1}}
+
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
+
+	w.DetectHostnameChanges(
+		t.Context(), host,
+		saved(map[string]*state.NameserverRecordState{nsA: answered(records)}),
+		saved(map[string]*state.NameserverRecordState{nsA: failed()}),
+	)
+
+	notifications := notifier.getNotifications()
+	if len(notifications) != 1 {
+		t.Fatalf("sent %v, want one NS Failure", notifications)
+	}
+
+	msg := notifications[0].Message
+	if !strings.Contains(msg, nsA) ||
+		!strings.Contains(msg, failed().Error) {
+		t.Errorf("message %q does not name %s and the reason", msg, nsA)
+	}
+}
+
+// TestNameserverThatNeverAnswers asks a nameserver address where
+// nothing answers, 192.0.2.1, and checks what the watcher saves for it.
+// The deadline outlasts the resolver's first two-second try, as in the
+// resolver's timeout test.
+func TestNameserverThatNeverAnswers(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second)
+	t.Cleanup(cancel)
+
+	res := resolver.NewFromLogger(slog.Default())
+
+	resp, err := res.QueryNameserverIP(ctx, nsA, "192.0.2.1", host)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	hs := watcher.BuildHostnameState(
+		map[string]*resolver.NameserverResponse{nsA: resp}, time.Now(),
+	)
+
+	got := hs.RecordsByNameserver[nsA]
+	if got.Status != "error" || got.Error == "" {
+		t.Errorf(
+			"saved status %q, error %q; want status error with a reason",
+			got.Status, got.Error,
+		)
+	}
+}

internal/watcher/watcher.go

@@ -13,6 +13,7 @@ import (
 
 	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/logger"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/state"
 	"sneak.berlin/go/dnswatcher/internal/tlscheck"
 )
@@ -249,7 +250,7 @@ func (w *Watcher) checkDomain(
 	// Also look up A/AAAA records for the apex domain so that
 	// port and TLS checks (which read HostnameState) can find
 	// the domain's IP addresses.
-	records, err := w.resolver.LookupAllRecords(ctx, domain)
+	results, err := w.resolver.LookupAllRecords(ctx, domain)
 	if err != nil {
 		w.log.Error(
 			"failed to lookup records for domain",
@@ -260,12 +261,13 @@ func (w *Watcher) checkDomain(
 		return
 	}
 
+	newState := buildHostnameState(results, now)
+
 	prevHS, hasPrevHS := w.state.GetHostnameState(domain)
 	if hasPrevHS && !w.firstRun {
-		w.detectHostnameChanges(ctx, domain, prevHS, records)
+		w.detectHostnameChanges(ctx, domain, prevHS, newState)
 	}
 
-	newState := buildHostnameState(records, now)
 	w.state.SetHostnameState(domain, newState)
 }
 
@@ -314,7 +316,7 @@ func (w *Watcher) checkHostname(
 	ctx context.Context,
 	hostname string,
 ) {
-	records, err := w.resolver.LookupAllRecords(ctx, hostname)
+	results, err := w.resolver.LookupAllRecords(ctx, hostname)
 	if err != nil {
 		w.log.Error(
 			"failed to lookup records",
@@ -325,19 +327,22 @@ func (w *Watcher) checkHostname(
 		return
 	}
 
-	now := time.Now().UTC()
+	newState := buildHostnameState(results, time.Now().UTC())
-	prev, hasPrev := w.state.GetHostnameState(hostname)
 
+	prev, hasPrev := w.state.GetHostnameState(hostname)
 	if hasPrev && !w.firstRun {
-		w.detectHostnameChanges(ctx, hostname, prev, records)
+		w.detectHostnameChanges(ctx, hostname, prev, newState)
 	}
 
-	newState := buildHostnameState(records, now)
 	w.state.SetHostnameState(hostname, newState)
 }
 
+// buildHostnameState saves each nameserver's response. A nameserver
+// that answered, even with NXDOMAIN or no records, is saved as ok; one
+// that timed out or failed is saved as error with the reason, and its
+// empty record set is not an answer.
 func buildHostnameState(
-	records map[string]map[string][]string,
+	results map[string]*resolver.NameserverResponse,
 	now time.Time,
 ) *state.HostnameState {
 	hs := &state.HostnameState{
@@ -347,12 +352,20 @@ func buildHostnameState(
 		LastChecked: now,
 	}
 
-	for ns, recs := range records {
+	for ns, resp := range results {
-		hs.RecordsByNameserver[ns] = &state.NameserverRecordState{
+		nsState := &state.NameserverRecordState{
-			Records:     recs,
+			Records:     resp.Records,
 			Status:      statusOK,
 			LastChecked: now,
 		}
+
+		if resp.Status == resolver.StatusTimeout ||
+			resp.Status == resolver.StatusError {
+			nsState.Status = statusError
+			nsState.Error = resp.Error
+		}
+
+		hs.RecordsByNameserver[ns] = nsState
 	}
 
 	return hs
@@ -361,27 +374,29 @@ func buildHostnameState(
 func (w *Watcher) detectHostnameChanges(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) {
 	w.detectRecordChanges(ctx, hostname, prev, current)
 	w.detectNSDisappearances(ctx, hostname, prev, current)
+	w.detectNSFailures(ctx, hostname, prev, current)
 	w.detectInconsistencies(ctx, hostname, prev, current)
 }
 
+// detectRecordChanges compares each nameserver's records with those of
+// the previous check. Only answers are compared: a nameserver that
+// failed on either check has no records to compare.
 func (w *Watcher) detectRecordChanges(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) {
-	for ns, recs := range current {
+	for ns, cur := range current.RecordsByNameserver {
 		prevNS, ok := prev.RecordsByNameserver[ns]
-		if !ok {
+		if !ok || prevNS.Status != statusOK || cur.Status != statusOK {
 			continue
 		}
 
-		if recordsEqual(prevNS.Records, recs) {
+		if recordsEqual(prevNS.Records, cur.Records) {
 			continue
 		}
 
@@ -389,7 +404,7 @@ func (w *Watcher) detectRecordChanges(
 			"Hostname: %s\nNameserver: %s\n"+
 				"Old: %v\nNew: %v",
 			hostname, ns,
-			prevNS.Records, recs,
+			prevNS.Records, cur.Records,
 		)
 
 		w.notify.SendNotification(
@@ -404,11 +419,10 @@ func (w *Watcher) detectRecordChanges(
 func (w *Watcher) detectNSDisappearances(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) {
 	for ns, prevNS := range prev.RecordsByNameserver {
-		if _, ok := current[ns]; ok || prevNS.Status != statusOK {
+		if _, ok := current.RecordsByNameserver[ns]; ok || prevNS.Status != statusOK {
 			continue
 		}
 
@@ -424,13 +438,36 @@ func (w *Watcher) detectNSDisappearances(
 			"error",
 		)
 	}
+}
 
-	for ns := range current {
+// detectNSFailures notifies when a nameserver that answered on the
+// previous check fails, and when one that failed answers again. A
+// nameserver missing from the previous check is not compared.
+func (w *Watcher) detectNSFailures(
+	ctx context.Context,
+	hostname string,
+	prev, current *state.HostnameState,
+) {
+	for ns, cur := range current.RecordsByNameserver {
 		prevNS, ok := prev.RecordsByNameserver[ns]
-		if !ok || prevNS.Status != statusError {
+		if !ok {
 			continue
 		}
 
+		switch {
+		case prevNS.Status == statusOK && cur.Status == statusError:
+			msg := fmt.Sprintf(
+				"Hostname: %s\nNameserver: %s\nError: %s",
+				hostname, ns, cur.Error,
+			)
+
+			w.notify.SendNotification(
+				ctx,
+				"NS Failure: "+hostname,
+				msg,
+				"error",
+			)
+		case prevNS.Status == statusError && cur.Status == statusOK:
 			msg := fmt.Sprintf(
 				"Hostname: %s\nNameserver: %s recovered",
 				hostname, ns,
@@ -443,13 +480,13 @@ func (w *Watcher) detectNSDisappearances(
 				"success",
 			)
 		}
+	}
 }
 
 func (w *Watcher) detectInconsistencies(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) {
 	for _, pair := range newlyDisagreeingPairs(prev, current) {
 		ns1, ns2 := pair[0], pair[1]
@@ -457,8 +494,8 @@ func (w *Watcher) detectInconsistencies(
 		msg := fmt.Sprintf(
 			"Hostname: %s\n%s: %v\n%s: %v",
 			hostname,
-			ns1, current[ns1],
+			ns1, current.RecordsByNameserver[ns1].Records,
-			ns2, current[ns2],
+			ns2, current.RecordsByNameserver[ns2].Records,
 		)
 
 		w.notify.SendNotification(
@@ -470,18 +507,21 @@ func (w *Watcher) detectInconsistencies(
 	}
 }
 
-// newlyDisagreeingPairs returns every pair of nameservers whose records
+// newlyDisagreeingPairs returns every pair of nameservers that answered
-// differ in current, in sorted order of name, except pairs where both
+// in current and whose records differ there, in sorted order of name,
-// nameservers were in prev and already differed there. A nameserver
+// except pairs where both nameservers answered in prev and already
-// missing from prev is paired with every nameserver it differs from.
+// differed there. A nameserver missing from prev, or that failed there,
+// is paired with every nameserver it differs from. A nameserver that
+// failed in current has no records to compare and is in no pair.
 func newlyDisagreeingPairs(
-	prev *state.HostnameState,
+	prev, current *state.HostnameState,
-	current map[string]map[string][]string,
 ) [][2]string {
-	nameservers := make([]string, 0, len(current))
+	nameservers := make([]string, 0, len(current.RecordsByNameserver))
-	for ns := range current {
+	for ns, cur := range current.RecordsByNameserver {
+		if cur.Status == statusOK {
 			nameservers = append(nameservers, ns)
 		}
+	}
 
 	sort.Strings(nameservers)
 
@@ -489,14 +529,19 @@ func newlyDisagreeingPairs(
 
 	for i, ns1 := range nameservers {
 		for _, ns2 := range nameservers[i+1:] {
-			if recordsEqual(current[ns1], current[ns2]) {
+			if recordsEqual(
+				current.RecordsByNameserver[ns1].Records,
+				current.RecordsByNameserver[ns2].Records,
+			) {
 				continue
 			}
 
 			prev1, ok1 := prev.RecordsByNameserver[ns1]
 			prev2, ok2 := prev.RecordsByNameserver[ns2]
 
-			if ok1 && ok2 && !recordsEqual(prev1.Records, prev2.Records) {
+			if ok1 && ok2 &&
+				prev1.Status == statusOK && prev2.Status == statusOK &&
+				!recordsEqual(prev1.Records, prev2.Records) {
 				continue
 			}
 

internal/watcher/watcher_test.go

@@ -666,11 +666,12 @@ func TestNSFailureAndRecovery(t *testing.T) {
 	cfg.Hostnames = []string{testHost}
 
 	// Between the checks, save every nameserver the first check found
-	// as failed, and add, as answering, one that live DNS does not list.
+	// as one that did not answer, and add, as answering, one that live
+	// DNS does not list, which then disappears.
 	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
 		hs, _ := deps.state.GetHostnameState(testHost)
-		for _, nsState := range hs.RecordsByNameserver {
+		for ns := range hs.RecordsByNameserver {
-			nsState.Status = "error"
+			hs.RecordsByNameserver[ns] = failed()
 		}
 
 		hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{
@@ -683,4 +684,10 @@ func TestNSFailureAndRecovery(t *testing.T) {
 
 	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
+	// its recovery is not also a record change.
+	if n := countNotifications(deps, "Record Change: "+testHost); n != 0 {
+		t.Errorf("sent %d record changes on recovery, want 0", n)
+	}
 }