From 52e4956777bc994b8ff904d14bc59c8ef6d375e2 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 2 Oct 2026 10:35:21 +0000 Subject: [PATCH] resolver: a nameserver with a failed record type is not nodata (closes #253) classifyResponse set nodata when every record type that answered had no records, even when another type's query got no usable reply. That type is listed in FailedTypes and its records are unknown, so the nameserver has not said it has none. It now stays ok, the status README describes for a nameserver with a failed type, and nodata is set only when no type failed. The watcher saved nodata as ok already, so saved state is unchanged; the live test that rejects nodata no longer fails when one of a nameserver's queries is lost. Model: opus-5-5 --- TODO.md | 2 + internal/resolver/iterative.go | 7 ++- internal/resolver/iterative_internal_test.go | 51 ++++++++++++++------ 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/TODO.md b/TODO.md index 85263d0..cc1b9cd 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-02: a nameserver whose query for one record type failed while the + others answered with no records is `ok`, not `nodata` (closes #253). - 2026-10-02: the refused-query test sends one query to four operators' public resolvers in turn until one replies, not eight to one operator (closes #251). - 2026-10-02: a name removed from `DNSWATCHER_TARGETS` leaves the state, and so diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 4b15543..f8a7911 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -913,7 +913,9 @@ func isTimeout(err error) bool { // classifyResponse sets the nameserver's status. One that answered no // record type has failed, and Error says why; one that answered some has -// the status of those answers. +// the status of those answers. It has no data only when every type +// answered with no records: a type in FailedTypes may have records, so a +// nameserver with one stays ok. func classifyResponse(resp *NameserverResponse, state queryState) { switch { case state.gotNXDomain && !state.hasRecords: @@ -933,7 +935,8 @@ func classifyResponse(resp *NameserverResponse, state queryState) { case state.gotReferral && !state.answered: resp.Status = StatusError resp.Error = "server returned a referral" - case !state.hasRecords && !state.gotNXDomain: + case !state.hasRecords && !state.gotNXDomain && + len(resp.FailedTypes) == 0: resp.Status = StatusNoData } } diff --git a/internal/resolver/iterative_internal_test.go b/internal/resolver/iterative_internal_test.go index f36097d..b1ae1e7 100644 --- a/internal/resolver/iterative_internal_test.go +++ b/internal/resolver/iterative_internal_test.go @@ -11,60 +11,77 @@ import ( ) // TestClassifyResponse sets a nameserver's status from the results of -// its queries, built here. One that answered some record types, even -// with no records, has not failed when its query for another type got -// no usable reply, whatever the reason; one whose every query got none -// has. +// its queries and the record types whose query failed, built here. One +// that answered some record types, even with no records, has not failed +// when its query for another type got no usable reply, whatever the +// reason, and is ok, not nodata: that type may have records. One whose +// every query got none has failed. Only one whose every type answered +// with no records is nodata. func TestClassifyResponse(t *testing.T) { t.Parallel() tests := []struct { - name string - results queryState - wantStatus string - wantError string + name string + results queryState + failedTypes []string + wantStatus string + wantError string }{ + { + "every type answered with no records", + queryState{answered: true}, + nil, + StatusNoData, "", + }, { "some types answered with no records, another timed out", queryState{answered: true, gotTimeout: true}, - StatusNoData, "", + []string{"A"}, + StatusOK, "", }, { "some types answered with no records, another got SERVFAIL", queryState{ answered: true, gotErrorReply: true, errorReply: "SERVFAIL", }, - StatusNoData, "", + []string{"A"}, + StatusOK, "", }, { "some types answered with no records, another was refused", queryState{answered: true, gotRefused: true}, - StatusNoData, "", + []string{"A"}, + StatusOK, "", }, { "some types answered with no records, another got a network error", queryState{answered: true, netErr: syscall.ECONNREFUSED}, - StatusNoData, "", + []string{"A"}, + StatusOK, "", }, { "some types answered with no records, another's reply was " + "truncated and its retry over TCP failed", queryState{answered: true, netErr: ErrTruncated}, - StatusNoData, "", + []string{"TXT"}, + StatusOK, "", }, { "some types answered with no records, another got a referral", queryState{answered: true, gotReferral: true}, - StatusNoData, "", + []string{"A"}, + StatusOK, "", }, { "every query timed out", queryState{gotTimeout: true}, + []string{"A", "AAAA", "CNAME"}, StatusTimeout, "all queries timed out", }, { "every query got NOTIMP", queryState{gotErrorReply: true, errorReply: "NOTIMP"}, + []string{"A", "AAAA", "CNAME"}, StatusError, "server returned NOTIMP", }, } @@ -73,11 +90,15 @@ func TestClassifyResponse(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - resp := &NameserverResponse{Status: StatusOK} + resp := &NameserverResponse{ + Status: StatusOK, + FailedTypes: tt.failedTypes, + } classifyResponse(resp, tt.results) assert.Equal(t, tt.wantStatus, resp.Status) assert.Equal(t, tt.wantError, resp.Error) + assert.Equal(t, tt.failedTypes, resp.FailedTypes) }) } }