watcher: a lookup cut short by shutdown is not logged as an error (closes #229) #237

Merged
clawbot merged 1 commits from issue-229-shutdown-lookup-not-error into next 2026-10-02 08:56:53 +02:00
4 changed files with 104 additions and 4 deletions
+2
View File
@@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
# Completed Steps # Completed Steps
- 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
+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"
"slices" "slices"
@@ -213,13 +214,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,
@@ -320,7 +337,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,
@@ -372,7 +390,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,
@@ -455,7 +474,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,