resolver: a nameserver is asked at each of its addresses in turn (closes #285)

A nameserver's queries went only to the first IPv4 address its name
resolves to, so one unreachable address made it fail whenever that
address came first in the answer. Each record type is now asked at the
addresses one at a time until one gives a usable reply, and the next
type is asked first at the address that gave it. A type that fails is
logged with the reason at each address, and a nameserver is reported as
timed out only when every query timed out. The README's DNS Resolution
Strategy says which addresses are asked.

Model: opus-5-5

internal/resolver/errors.go

@@ -36,10 +36,6 @@ var (
 	// so whether the name has addresses is unknown.
 	ErrNoNameserverAnswered = errors.New("no nameserver answered")
 
-	// ErrNoNameserverAddress is returned when a nameserver is to be
-	// asked and no address for it was given.
-	ErrNoNameserverAddress = errors.New("nameserver has no address")
-
 	// ErrUnusableReply is returned when a server replied with an
 	// error such as SERVFAIL, or with a referral that leads no
 	// closer to the name asked about.

internal/resolver/iterative.go

@@ -752,8 +752,7 @@ func (r *Resolver) QueryNameserverIP(
 
 // queryTypes asks the nameserver nsHostname, at its addresses nsIPs,
 // about hostname's records of each type in qtypes and builds a
-// NameserverResponse. With no address there is nothing to ask, and it
+// NameserverResponse.
-// returns ErrNoNameserverAddress.
 func (r *Resolver) queryTypes(
 	ctx context.Context,
 	nsHostname string,
@@ -761,10 +760,6 @@ func (r *Resolver) queryTypes(
 	hostname string,
 	qtypes []uint16,
 ) (*NameserverResponse, error) {
-	if len(nsIPs) == 0 {
-		return nil, fmt.Errorf("%w: %s", ErrNoNameserverAddress, nsHostname)
-	}
-
 	resp := &NameserverResponse{
 		Nameserver: nsHostname,
 		Records:    make(map[string][]string),
@@ -854,10 +849,10 @@ func (r *Resolver) queryEachType(
 }
 
 // querySingleType asks the nameserver about hostname's records of type
-// qtype at its addresses nsIPs, of which there is at least one, one at a
+// qtype at its addresses nsIPs, one at a time, until one answers: with
-// time, until one answers: with records, with none, or with NXDOMAIN.
+// records, with none, or with NXDOMAIN. It returns the index in nsIPs of
-// It returns the index in nsIPs of the address that answered; when none
+// the address that answered; when none did, it returns why each did not,
-// did, it returns why each did not, each reason naming its address.
+// each reason naming its address.
 func (r *Resolver) querySingleType(
 	ctx context.Context,
 	nsIPs []string,
@@ -866,6 +861,10 @@ func (r *Resolver) querySingleType(
 	resp *NameserverResponse,
 	state *queryState,
 ) (int, error) {
+	if len(nsIPs) == 0 {
+		return 0, ErrNoNameserverAnswered
+	}
+
 	var errs []error
 
 	for i, nsIP := range nsIPs {

internal/resolver/iterative_internal_test.go

@@ -15,10 +15,8 @@ import (
 // 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, and its error says all queries timed
+// every query got none has failed. Only one whose every type answered
-// out only when every one did: a reply such as a refusal, or a network
+// with no records is nodata.
-// error, at one of its addresses is given instead. Only one whose every
-// type answered with no records is nodata.
 func TestClassifyResponse(t *testing.T) {
 	t.Parallel()
 
@@ -29,46 +27,63 @@ func TestClassifyResponse(t *testing.T) {
 		wantStatus  string
 		wantError   string
 	}{
-		{"every type answered with no records",
+		{
+			"every type answered with no records",
 			queryState{answered: true},
-			nil, StatusNoData, ""},
+			nil,
-		{"some types answered with no records, another timed out",
+			StatusNoData, "",
+		},
+		{
+			"some types answered with no records, another timed out",
 			queryState{answered: true, gotTimeout: true},
-			[]string{"A"}, StatusOK, ""},
+			[]string{"A"},
-		{"some types answered with no records, another got SERVFAIL",
+			StatusOK, "",
-			queryState{answered: true, gotErrorReply: true, errorReply: "SERVFAIL"},
+		},
-			[]string{"A"}, StatusOK, ""},
+		{
-		{"some types answered with no records, another was refused",
+			"some types answered with no records, another got SERVFAIL",
+			queryState{
+				answered: true, gotErrorReply: true, errorReply: "SERVFAIL",
+			},
+			[]string{"A"},
+			StatusOK, "",
+		},
+		{
+			"some types answered with no records, another was refused",
 			queryState{answered: true, gotRefused: true},
-			[]string{"A"}, StatusOK, ""},
+			[]string{"A"},
-		{"some types answered with no records, another got a network error",
+			StatusOK, "",
+		},
+		{
+			"some types answered with no records, another got a network error",
 			queryState{answered: true, netErr: syscall.ECONNREFUSED},
-			[]string{"A"}, StatusOK, ""},
+			[]string{"A"},
-		{"some types answered with no records, another's reply was " +
+			StatusOK, "",
-			"truncated and its retry over TCP failed",
+		},
+		{
+			"some types answered with no records, another's reply was " +
+				"truncated and its retry over TCP failed",
 			queryState{answered: true, netErr: ErrTruncated},
-			[]string{"TXT"}, StatusOK, ""},
+			[]string{"TXT"},
-		{"some types answered with no records, another got a referral",
+			StatusOK, "",
+		},
+		{
+			"some types answered with no records, another got a referral",
 			queryState{answered: true, gotReferral: true},
-			[]string{"A"}, StatusOK, ""},
+			[]string{"A"},
-		{"every query timed out",
+			StatusOK, "",
+		},
+		{
+			"every query timed out",
 			queryState{gotTimeout: true},
-			[]string{"A", "AAAA", "CNAME"}, StatusTimeout, "all queries timed out"},
+			[]string{"A", "AAAA", "CNAME"},
-		{"every query got NOTIMP",
+			StatusTimeout, "all queries timed out",
+		},
+		{
+			"every query got NOTIMP",
 			queryState{gotErrorReply: true, errorReply: "NOTIMP"},
-			[]string{"A", "AAAA", "CNAME"}, StatusError, "server returned NOTIMP"},
+			[]string{"A", "AAAA", "CNAME"},
-		{"every query got SERVFAIL at one address, timed out at another",
+			StatusError, "server returned NOTIMP",
-			queryState{gotErrorReply: true, errorReply: "SERVFAIL", gotTimeout: true},
+		},
-			[]string{"A"}, StatusError, "server returned SERVFAIL"},
-		{"every query was refused at one address, timed out at another",
-			queryState{gotRefused: true, gotTimeout: true},
-			[]string{"A"}, StatusError, "server returned REFUSED"},
-		{"every query got a network error at one address, timed out at another",
-			queryState{netErr: syscall.ECONNREFUSED, gotTimeout: true},
-			[]string{"A"}, StatusError, "network error: connection refused"},
-		{"every query got a referral at one address, timed out at another",
-			queryState{gotReferral: true, gotTimeout: true},
-			[]string{"A"}, StatusError, "server returned a referral"},
 	}
 
 	for _, tt := range tests {
@@ -85,6 +100,20 @@ func TestClassifyResponse(t *testing.T) {
 	}
 }
 
+// TestClassifyResponse_RefusedAndTimedOut sets the status of a
+// nameserver one of whose addresses refused every query while another
+// never replied. It has failed with the refusal: not every query timed
+// out.
+func TestClassifyResponse_RefusedAndTimedOut(t *testing.T) {
+	t.Parallel()
+
+	resp := &NameserverResponse{Status: StatusOK, FailedTypes: []string{"A"}}
+	classifyResponse(resp, queryState{gotRefused: true, gotTimeout: true})
+
+	assert.Equal(t, StatusError, resp.Status)
+	assert.Equal(t, "server returned REFUSED", resp.Error)
+}
+
 // TestReadReply checks which replies to a query about one record type,
 // built here, are an answer: one with the code NOERROR or NXDOMAIN. A
 // reply with any other code is not, and the type's query has failed; a

internal/resolver/resolver_test.go

@@ -464,20 +464,6 @@ func TestQueryTypes_EveryAddressFails(t *testing.T) {
 	assert.Contains(t, logs.String(), "@"+nsIPs[1]+": ")
 }
 
-// TestQueryTypes_NoAddress asks a nameserver given no address. There is
-// nothing to ask it at, so it has failed, with an error that says it has
-// no address; it is not ok.
-func TestQueryTypes_NoAddress(t *testing.T) {
-	t.Parallel()
-
-	resp, err := newTestResolver(t).QueryTypes(
-		t.Context(), "ns1.example.net.", nil, "www.example.net.",
-		[]uint16{dns.TypeA},
-	)
-	require.ErrorIs(t, err, resolver.ErrNoNameserverAddress)
-	assert.Nil(t, resp)
-}
-
 func TestQueryNameserver_AAAA(t *testing.T) {
 	t.Parallel()
 
@@ -542,9 +528,8 @@ func TestQueryNameserver_TXT(t *testing.T) {
 // nameserver about google.com with a resolver whose retries over TCP
 // fail. google.com's TXT records do not fit in a reply over UDP, so TXT
 // is reported as failed, holding none of the records that fit, and
-// logged with the reason, which names the address whose reply was
+// logged with the reason, while the nameserver, which answered the other
-// truncated, while the nameserver, which answered the other types, is
+// types, is ok.
-// ok.
 func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) {
 	t.Parallel()
 
@@ -561,13 +546,6 @@ func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) {
 	assert.Contains(t, logs.String(),
 		"hostname=google.com. nameserver="+ns+" type=TXT error=",
 	)
-
-	nsIPs := liveResolveNSIPs(t, newTestResolver(t), []string{ns}, 1)
-	for _, ip := range nsIPs {
-		assert.Contains(t, logs.String(),
-			"@"+ip+": "+resolver.ErrTruncated.Error(),
-		)
-	}
 }
 
 func TestQueryNameserver_NXDomain(t *testing.T) {