2 Commits
Author SHA1 Message Date
sneak d735de8746 resolver, watcher: a record type whose query fails keeps its previous records (closes #231)
check / check (push) Canceled after 0s
The resolver lists in FailedTypes each record type whose query to a
nameserver got no usable reply (none after two tries, an error reply, a
referral, or a truncated reply whose TCP retry failed), and logs it with
the reason unless shutdown cut it short. A nameserver that answered no
type has failed, as before.
The watcher saves such a type in failedTypes, keeping the previous
check's records, leaves it out of the comparison with other nameservers
on that check, and compares the kept records with the next answer. With
nothing to keep, it is also in unknownTypes and not compared until it
answers. Change messages leave out what was not compared. A nameserver
whose A, AAAA or CNAME query failed is no answer when following a CNAME
or resolving addresses.

Model: opus-5-5
2026-10-02 07:06:42 +00:00
clawbot b047c3c64c watcher: a lookup cut short by shutdown is not logged as an error (closes #229)
check / check (push) Canceled after 0s
Stopping dnswatcher during a DNS check logged every lookup the stop cut
short as an error, "context canceled". The four places in the watcher
that log a failed lookup, checkDomain, checkHostname,
resolveNameserverAddresses and resolveCNAMEAddresses, now do it through
logFailedLookup, which logs nothing when the watcher's context was
cancelled. It asks the context, not the lookup's error: the resolver
reports a cancelled lookup with its own error, which does not wrap
context.Canceled. A context whose deadline passed is not cancelled, so a
lookup that ran out of time is still logged. The tests run each of these
lookups on a cancelled context and on one whose deadline passed; neither
sends a query.

Model: opus-5-5
2026-10-02 08:56:52 +02:00
6 changed files with 133 additions and 5 deletions
+2
View File
@@ -21,6 +21,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
- 2026-10-02: a record type whose query to a nameserver fails keeps its previous - 2026-10-02: a record type whose query to a nameserver fails keeps its previous
records and alerts nothing; the other types are still saved (closes #231). records and alerts nothing; the other types are still saved (closes #231).
- 2026-10-02: a DNS lookup that shutdown cuts short logs no error; one that
fails otherwise, or runs out of time, still does (closes #229).
- 2026-10-02: Record Change and Inconsistency notifications list only the record - 2026-10-02: Record Change and Inconsistency notifications list only the record
types that differ, each with its values as plain text (closes #219). types that differ, each with its values as plain text (closes #219).
- 2026-10-02: the startup notification no longer says every notification - 2026-10-02: the startup notification no longer says every notification
+6 -1
View File
@@ -646,7 +646,8 @@ type queryState struct {
// queryEachType asks the nameserver at nsIP about hostname once for each // queryEachType asks the nameserver at nsIP about hostname once for each
// record type in qtypes, and lists in resp.FailedTypes the types whose // record type in qtypes, and lists in resp.FailedTypes the types whose
// query got no usable reply, logging each with the reason. // query got no usable reply, logging each with the reason unless ctx was
// cancelled: shutdown cancels it, and a query it cut short did not fail.
func (r *Resolver) queryEachType( func (r *Resolver) queryEachType(
ctx context.Context, ctx context.Context,
nsIP string, nsIP string,
@@ -671,6 +672,10 @@ func (r *Resolver) queryEachType(
rtype := dns.TypeToString[qtype] rtype := dns.TypeToString[qtype]
resp.FailedTypes = append(resp.FailedTypes, rtype) resp.FailedTypes = append(resp.FailedTypes, rtype)
if errors.Is(ctx.Err(), context.Canceled) {
continue
}
r.log.Warn( r.log.Warn(
"record type query failed", "record type query failed",
"hostname", hostname, "hostname", hostname,
+23
View File
@@ -890,6 +890,29 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
assert.NotEmpty(t, resp.Error) assert.NotEmpty(t, resp.Error)
} }
// TestQueryNameserverIP_CancelledLogsNothing cancels the context while
// a query to 192.0.2.1, where nothing answers, is waiting for a reply,
// as shutdown does. The query was cut short, not failed, so nothing is
// logged.
func TestQueryNameserverIP_CancelledLogsNothing(t *testing.T) {
t.Parallel()
var logs bytes.Buffer
r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil)))
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
time.AfterFunc(100*time.Millisecond, cancel)
_, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1", "example.com",
)
require.NoError(t, err)
assert.Empty(t, logs.String())
}
// TestCollectIPs_NoNameserverAnswered takes the response of a // TestCollectIPs_NoNameserverAnswered takes the response of a
// nameserver at 192.0.2.1, where nothing answers, as // nameserver at 192.0.2.1, where nothing answers, as
// TestQueryNameserverIP_Timeout does. Addresses collected from // TestQueryNameserverIP_Timeout does. Addresses collected from
+72
View File
@@ -1,10 +1,13 @@
package watcher_test package watcher_test
import ( import (
"bytes"
"context" "context"
"log/slog" "log/slog"
"reflect" "reflect"
"strings"
"testing" "testing"
"time"
"sneak.berlin/go/dnswatcher/internal/portcheck" "sneak.berlin/go/dnswatcher/internal/portcheck"
"sneak.berlin/go/dnswatcher/internal/resolver" "sneak.berlin/go/dnswatcher/internal/resolver"
@@ -79,3 +82,72 @@ func TestCancelledCheckSavesNothing(t *testing.T) {
t.Errorf("sent %v, want no notifications", notifications) t.Errorf("sent %v, want no notifications", notifications)
} }
} }
// newLoggingWatcher returns a watcher for a domain and a hostname, with
// the real resolver, that writes what it logs at warning level or above
// into the returned buffer.
func newLoggingWatcher(t *testing.T) (*watcher.Watcher, *bytes.Buffer) {
t.Helper()
cfg := defaultTestConfig(t)
cfg.Domains = []string{testSmallDomain}
cfg.Hostnames = []string{host}
w, _ := newTestWatcher(t, cfg)
logs := &bytes.Buffer{}
w.SetLogger(slog.New(slog.NewJSONHandler(
logs, &slog.HandlerOptions{Level: slog.LevelWarn},
)))
return w, logs
}
// TestLookupCutShortIsNotLogged checks a domain and a hostname, looks
// up a nameserver's addresses and follows a CNAME, with the context
// cancelled, as shutdown leaves it. The real resolver fails each lookup
// without sending a query. Shutdown cutting a lookup short is not a
// failure, so nothing may be logged at warning level or above.
func TestLookupCutShortIsNotLogged(t *testing.T) {
t.Parallel()
w, logs := newLoggingWatcher(t)
ctx, cancel := context.WithCancel(t.Context())
cancel()
w.RunOnce(ctx)
w.ResolveNameserverAddresses(ctx, []string{nsA}, nil)
w.ResolveCNAMEAddresses(ctx, host, cnameState(), nil)
if logs.Len() > 0 {
t.Errorf("logged at warning level or above:\n%s", logs)
}
}
// TestLookupOutOfTimeIsLoggedAsError does what
// TestLookupCutShortIsNotLogged does, with the context's deadline passed
// instead. A lookup that ran out of time did fail, so the domain's NS
// lookup, the hostname's lookup, the nameserver's address lookup and the
// CNAME's are each logged as an error.
func TestLookupOutOfTimeIsLoggedAsError(t *testing.T) {
t.Parallel()
w, logs := newLoggingWatcher(t)
ctx, cancel := context.WithDeadline(t.Context(), time.Now())
t.Cleanup(cancel)
w.RunOnce(ctx)
w.ResolveNameserverAddresses(ctx, []string{nsA}, nil)
w.ResolveCNAMEAddresses(ctx, host, cnameState(), nil)
const want = 4
lines := strings.Count(logs.String(), "\n")
errorLines := strings.Count(logs.String(), `"level":"ERROR"`)
if lines != want || errorLines != want {
t.Errorf("logged:\n%s\nwant %d lines, each at error level", logs, want)
}
}
+6
View File
@@ -31,6 +31,12 @@ func NewForTest(
} }
} }
// SetLogger replaces the watcher's logger, so a test can read what it
// logs.
func (w *Watcher) SetLogger(log *slog.Logger) {
w.log = log
}
// NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing. // NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing.
func NewlyDisagreeingPairs( func NewlyDisagreeingPairs(
prev, current *state.HostnameState, prev, current *state.HostnameState,
+24 -4
View File
@@ -2,6 +2,7 @@ package watcher
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"maps" "maps"
@@ -214,13 +215,29 @@ func (w *Watcher) runDNSChecks(ctx context.Context) {
} }
} }
// logFailedLookup logs a failed DNS lookup at error level, unless ctx
// was cancelled: shutdown cancels it, and a lookup it cut short did not
// fail. A lookup that ran out of time did fail, so it is logged.
func (w *Watcher) logFailedLookup(
ctx context.Context,
msg string,
args ...any,
) {
if errors.Is(ctx.Err(), context.Canceled) {
return
}
w.log.Error(msg, args...)
}
func (w *Watcher) checkDomain( func (w *Watcher) checkDomain(
ctx context.Context, ctx context.Context,
domain string, domain string,
) { ) {
nameservers, err := w.resolver.LookupNS(ctx, domain) nameservers, err := w.resolver.LookupNS(ctx, domain)
if err != nil { if err != nil {
w.log.Error( w.logFailedLookup(
ctx,
"failed to lookup NS", "failed to lookup NS",
"domain", domain, "domain", domain,
"error", err, "error", err,
@@ -321,7 +338,8 @@ func (w *Watcher) resolveNameserverAddresses(
continue continue
} }
w.log.Error( w.logFailedLookup(
ctx,
"no addresses found for nameserver", "no addresses found for nameserver",
"nameserver", ns, "nameserver", ns,
"error", err, "error", err,
@@ -373,7 +391,8 @@ func (w *Watcher) checkHostname(
) { ) {
results, err := w.resolver.LookupAllRecords(ctx, hostname) results, err := w.resolver.LookupAllRecords(ctx, hostname)
if err != nil { if err != nil {
w.log.Error( w.logFailedLookup(
ctx,
"failed to lookup records", "failed to lookup records",
"hostname", hostname, "hostname", hostname,
"error", err, "error", err,
@@ -462,7 +481,8 @@ func (w *Watcher) resolveCNAMEAddresses(
for target := range targets { for target := range targets {
ips, err := w.resolver.ResolveIPAddresses(ctx, target) ips, err := w.resolver.ResolveIPAddresses(ctx, target)
if err != nil { if err != nil {
w.log.Error( w.logFailedLookup(
ctx,
"failed to follow CNAME", "failed to follow CNAME",
"hostname", hostname, "hostname", hostname,
"target", target, "target", target,