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. The README's DNS
Resolution Strategy says which addresses are asked.

Model: opus-5-5

internal/resolver/iterative.go

@@ -784,11 +784,11 @@ type queryState struct {
 	answered      bool
 }
 
-// queryEachType asks the nameserver about hostname's records of each type
-// in qtypes, at its addresses nsIPs in turn until one gives a usable
-// reply, and lists in resp.FailedTypes the types that got none at any of
-// them, logging each with the reasons unless ctx was cancelled: shutdown
-// cancels it, and a query it cut short did not fail.
+// queryEachType asks the nameserver at its addresses nsIPs about hostname
+// once for each record type in qtypes, and lists in resp.FailedTypes the
+// types whose query got no usable reply at any of them, 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(
 	ctx context.Context,
 	nsIPs []string,
@@ -851,8 +851,8 @@ func (r *Resolver) queryEachType(
 // querySingleType asks the nameserver about hostname's records of type
 // qtype at its addresses nsIPs, one at a time, until one answers: with
 // records, with none, or with NXDOMAIN. It returns the index in nsIPs of
-// the address that answered; when none did, it returns why each did not,
-// each reason naming its address.
+// the address that answered; when none did, it returns why the last one
+// did not.
 func (r *Resolver) querySingleType(
 	ctx context.Context,
 	nsIPs []string,
@@ -861,21 +861,16 @@ func (r *Resolver) querySingleType(
 	resp *NameserverResponse,
 	state *queryState,
 ) (int, error) {
-	if len(nsIPs) == 0 {
-		return 0, ErrNoNameserverAnswered
-	}
-
-	var errs []error
+	var err error
 
 	for i, nsIP := range nsIPs {
-		msg, err := r.queryDNS(ctx, nsIP, hostname, qtype)
+		var msg *dns.Msg
+
+		msg, err = r.queryDNS(ctx, nsIP, hostname, qtype)
 
 		switch {
 		case err == nil:
 			err = readReply(msg, resp, state)
-			if err != nil {
-				err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
-			}
 		case isTimeout(err):
 			state.gotTimeout = true
 		case errors.Is(err, ErrRefused):
@@ -887,11 +882,9 @@ func (r *Resolver) querySingleType(
 		if err == nil {
 			return i, nil
 		}
-
-		errs = append(errs, err)
 	}
 
-	return 0, errors.Join(errs...)
+	return 0, err
 }
 
 // readReply adds to resp the records in msg, a nameserver's reply to a
@@ -985,15 +978,17 @@ func isTimeout(err error) bool {
 }
 
 // classifyResponse sets the nameserver's status. One that answered no
-// record type has failed, and Error says why; it says all queries timed
-// out only when every query, at every address, did. One that answered
-// some has the status of those answers. It has no data only when every type
+// record type has failed, and Error says why; one that answered some has
+// 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:
 		resp.Status = StatusNXDomain
+	case state.gotTimeout && !state.answered:
+		resp.Status = StatusTimeout
+		resp.Error = "all queries timed out"
 	case state.gotErrorReply && !state.answered:
 		resp.Status = StatusError
 		resp.Error = "server returned " + state.errorReply
@@ -1006,9 +1001,6 @@ func classifyResponse(resp *NameserverResponse, state queryState) {
 	case state.gotReferral && !state.answered:
 		resp.Status = StatusError
 		resp.Error = "server returned a referral"
-	case state.gotTimeout && !state.answered:
-		resp.Status = StatusTimeout
-		resp.Error = "all queries timed out"
 	// An NXDOMAIN reply with no records was taken by the first case.
 	case !state.hasRecords && len(resp.FailedTypes) == 0:
 		resp.Status = StatusNoData

internal/resolver/iterative_internal_test.go

@@ -100,20 +100,6 @@ 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

@@ -412,58 +412,6 @@ func TestQueryTypes_FirstAddressNeverAnswers(t *testing.T) {
 	assert.NotEmpty(t, resp.Records["AAAA"])
 }
 
-// TestQueryTypes_EveryAddressFails asks a google.com nameserver about
-// cloudflare.com, a zone it does not serve, at 192.0.2.1, where nothing
-// answers, and then at its own address, which refuses. The nameserver
-// has failed with the refusal, not a timeout, and the warning about the
-// type gives the reason at each address.
-func TestQueryTypes_EveryAddressFails(t *testing.T) {
-	t.Parallel()
-
-	var logs bytes.Buffer
-
-	r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil)))
-	ns := findOneNSForDomain(t, r, "google.com")
-	nsIPs := append(
-		[]string{noAnswerAddress}, liveResolveNSIPs(t, r, []string{ns}, 1)...,
-	)
-
-	var resp *resolver.NameserverResponse
-
-	livednstest.Retry(
-		t,
-		"QueryTypes("+ns+" at "+strings.Join(nsIPs, ", ")+", cloudflare.com)",
-		func(ctx context.Context) error {
-			logs.Reset()
-
-			var err error
-
-			resp, err = r.QueryTypes(
-				ctx, ns, nsIPs, "cloudflare.com.", []uint16{dns.TypeA},
-			)
-			if err != nil {
-				return err
-			}
-
-			// A timeout or a network error at its own address 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)
-	assert.Contains(t, logs.String(), "@"+noAnswerAddress+": ")
-	assert.Contains(t, logs.String(), "@"+nsIPs[1]+": ")
-}
-
 func TestQueryNameserver_AAAA(t *testing.T) {
 	t.Parallel()