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; one given no address has
failed. The README's DNS Resolution Strategy says which addresses are
asked.

Model: opus-5-5

README.md

@@ -286,7 +286,7 @@ includes:
 
 Each endpoint is sent each notification on its own, in the background. A
 delivery that fails (a network error, no reply within 10 seconds, or an HTTP
-status other than 2xx) is retried up to 5 times: the first retry after about 1
+status of 400 or more) is retried up to 5 times: the first retry after about 1
 second, each wait after that twice as long up to 60 seconds, every wait varied
 at random by up to 25%. A delivery still failing after that is logged and
 dropped.
@@ -910,7 +910,6 @@ internal/
     templates/dashboard.html   Dashboard template (embedded)
     status.go                  /api/v1/status
     healthcheck.go             Health check handler
-    static.go                  Static assets under /s/
   server/
     server.go                  HTTP server lifecycle
     routes.go                  Route definitions

TESTING.md

@@ -19,18 +19,16 @@ works correctly in production.
 ### Constraints
 
 - Tests hit real DNS infrastructure and require network access
-- Test duration depends on network conditions. `make test` must finish in under
+- Test duration depends on network conditions; timeout tuning keeps the suite
-  60 seconds, the hard cap in `REPO_POLICIES.md`; under 20 seconds is the target
+  within the 60-second target
-- Each query to a nameserver has a 2-second timeout, and a query over UDP that
+- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus
-  gets no reply is sent once more (`queryTimeoutDuration` and `maxRetries` in
+  processing margin
-  `internal/resolver/iterative.go`)
+- Root server fan-out is limited to reduce parallel query load
-- In the walk to a name's nameservers, the servers of each zone are asked one at
-  a time in a random order, as the README's DNS Resolution Strategy describes
 - Live lookups that expect an answer go through `internal/livednstest`, which
   limits how many run at once in a test binary and retries a lookup that got
   none
-- A flaky live test is a robustness problem, fixed with retries with backoff,
+- Flaky failures from transient network issues are acceptable and should be
-  several independent nameservers or longer timeouts, never with mocks or skip
+  investigated as potential resolver bugs, not papered over with mocks or skip
   flags
 
 ### What NOT to do

TODO.md

@@ -19,21 +19,6 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Completed Steps
 
-- 2026-10-15: a query its caller's context ends before it is sent records no
-  reason about the nameserver, and a try the context's deadline cuts short is a
-  timeout, so a nameserver whose only failures were timeouts is `timeout`, not a
-  network error or `ok` (closes #298).
-- 2026-10-15: the resolver no longer says its query timeout grows on each try;
-  every try waits the same 2 seconds, and the unused `timeoutMultiplier` is gone
-  (closes #296).
-- 2026-10-15: `TESTING.md` Constraints give the resolver's 2-second query
-  timeout and second try, say a zone's servers are asked one at a time in a
-  random order, give the 60-second cap and 20-second target for `make test`, and
-  say a flaky live test is fixed with robustness, not with mocks (closes #289).
-- 2026-10-15: a request under `/s/` that names a directory, such as `/s/` or
-  `/s/css`, answers 404 instead of a listing of its files (closes #286).
-- 2026-10-15: a notification endpoint that answers with a redirect, or any
-  status other than 2xx, is a failed delivery, retried and logged (closes #287).
 - 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it
   has failed only when none of them gives a usable reply (closes #285).
 - 2026-10-15: the log names a notification endpoint (`slack`, `mattermost`,

internal/handlers/static.go

@@ -1,36 +0,0 @@
-package handlers
-
-import (
-	"io/fs"
-	"net/http"
-	"strings"
-
-	"sneak.berlin/go/dnswatcher/static"
-)
-
-// HandleStatic returns the handler for the embedded static assets. It
-// is mounted under /s, with that prefix stripped from the path. A
-// request that names a directory answers 404: http.FileServer would
-// answer it with a listing of the directory, or with a redirect to
-// that listing when the trailing slash is missing.
-func (h *Handlers) HandleStatic() http.HandlerFunc {
-	files := http.FileServer(http.FS(static.Static))
-
-	return func(
-		writer http.ResponseWriter,
-		request *http.Request,
-	) {
-		// fs.Stat refuses a name that is empty or ends in a slash,
-		// so /s/ and /s/css/ answer 404 here along with /s/css.
-		name := strings.TrimPrefix(request.URL.Path, "/")
-
-		info, err := fs.Stat(static.Static, name)
-		if err != nil || info.IsDir() {
-			http.NotFound(writer, request)
-
-			return
-		}
-
-		files.ServeHTTP(writer, request)
-	}
-}

internal/notify/delivery_test.go

@@ -1185,42 +1185,6 @@ func sendToEachEndpoint(t *testing.T, baseURL, title string) string {
 	return logs.String()
 }
 
-// assertEachEndpointRetriedThenFailed fails the test unless, for each
-// of ntfy, Slack and Mattermost, the lines of logs whose error
-// contains reason are the retry line and then the final failure line.
-func assertEachEndpointRetriedThenFailed(
-	t *testing.T, logs, reason string,
-) {
-	t.Helper()
-
-	// For each endpoint, the messages of its lines whose error
-	// contains reason, in the order they were logged.
-	got := map[string][]string{}
-
-	for _, record := range parseLog(t, logs) {
-		if strings.Contains(record.Error, reason) {
-			got[record.Endpoint] = append(
-				got[record.Endpoint], record.Msg,
-			)
-		}
-	}
-
-	lines := []string{
-		"notification delivery failed, retrying",
-		"failed to send notification after retries",
-	}
-	want := map[string][]string{
-		endpointNtfy:       lines,
-		endpointSlack:      lines,
-		endpointMattermost: lines,
-	}
-
-	if !maps.EqualFunc(got, want, slices.Equal) {
-		t.Errorf("lines whose error contains %q = %v, want %v",
-			reason, got, want)
-	}
-}
-
 // TestSendNotificationDebugLogOmitsWebhookURL checks that, with debug
 // logging on, a delivery to each endpoint logs the endpoint's name and
 // the title, and no part of the URL's path or query.
@@ -1276,7 +1240,33 @@ func TestSendNotificationErrorLogOmitsWebhookURL(t *testing.T) {
 	logs := sendToEachEndpoint(t, srv.URL, "t")
 
 	assertNoWebhookSecret(t, logs)
-	assertEachEndpointRetriedThenFailed(t, logs, "connection refused")
+
+	// For each endpoint, the messages of its lines whose error names
+	// the refused connection, in the order they were logged.
+	got := map[string][]string{}
+
+	for _, record := range parseLog(t, logs) {
+		if strings.Contains(record.Error, "connection refused") {
+			got[record.Endpoint] = append(
+				got[record.Endpoint], record.Msg,
+			)
+		}
+	}
+
+	lines := []string{
+		"notification delivery failed, retrying",
+		"failed to send notification after retries",
+	}
+	want := map[string][]string{
+		endpointNtfy:       lines,
+		endpointSlack:      lines,
+		endpointMattermost: lines,
+	}
+
+	if !maps.EqualFunc(got, want, slices.Equal) {
+		t.Errorf("lines whose error names the refused connection = %v, "+
+			"want %v", got, want)
+	}
 }
 
 // ── SlackPayload JSON marshaling ──────────────────────────

internal/notify/notify.go

@@ -25,6 +25,9 @@ import (
 // HTTP client timeout.
 const httpClientTimeout = 10 * time.Second
 
+// HTTP status code thresholds.
+const httpStatusClientError = 400
+
 // Sentinel errors for notification failures.
 var (
 	// ErrNtfyFailed indicates the ntfy request failed.
@@ -341,10 +344,7 @@ func (svc *Service) sendNtfy(
 
 	defer func() { _ = resp.Body.Close() }()
 
-	// Only a 2xx reply means ntfy took the notification. A
+	if resp.StatusCode >= httpStatusClientError {
-	// redirect is not followed, so it is a failure too.
-	if resp.StatusCode < http.StatusOK ||
-		resp.StatusCode >= http.StatusMultipleChoices {
 		return fmt.Errorf(
 			"%w: status %d",
 			ErrNtfyFailed, resp.StatusCode,
@@ -385,9 +385,9 @@ type SlackAttachment struct {
 // sendSlack posts to a Slack or Mattermost incoming webhook, which
 // take the same payload. endpoint, "slack" or "mattermost", names
 // the webhook in the log in place of its URL, which lets anyone who
-// has it post to the channel. A reply status other than 2xx is
+// has it post to the channel. An HTTP error status is returned
-// returned wrapped in failed, ErrSlackFailed or ErrMattermostFailed,
+// wrapped in failed, ErrSlackFailed or ErrMattermostFailed, so the
-// so the error names the endpoint.
+// error names the endpoint.
 func (svc *Service) sendSlack(
 	ctx context.Context,
 	webhookURL *url.URL,
@@ -437,10 +437,7 @@ func (svc *Service) sendSlack(
 
 	defer func() { _ = resp.Body.Close() }()
 
-	// Only a 2xx reply means the webhook took the notification. A
+	if resp.StatusCode >= httpStatusClientError {
-	// redirect is not followed, so it is a failure too.
-	if resp.StatusCode < http.StatusOK ||
-		resp.StatusCode >= http.StatusMultipleChoices {
 		return fmt.Errorf(
 			"%w: status %d",
 			failed, resp.StatusCode,

internal/notify/retry_test.go

@@ -536,33 +536,3 @@ func TestSendNotificationPermanentFailureLogsError(
 		return attempts >= 3
 	})
 }
-
-// TestSendNotificationRedirectFails checks that a 302 reply is a
-// failed delivery at each endpoint: the server is sent the first
-// attempt and the retry, and each endpoint logs a retry line and
-// then the final failure line, both naming the 302.
-func TestSendNotificationRedirectFails(t *testing.T) {
-	t.Parallel()
-
-	var requests atomic.Int64
-
-	srv := httptest.NewServer(
-		http.HandlerFunc(
-			func(w http.ResponseWriter, r *http.Request) {
-				requests.Add(1)
-				http.Redirect(w, r, "/moved", http.StatusFound)
-			}),
-	)
-	defer srv.Close()
-
-	logs := sendToEachEndpoint(t, srv.URL, "t")
-
-	// ntfy, Slack and Mattermost each send once and retry once.
-	const wantRequests = 3 * 2
-
-	if sent := requests.Load(); sent != wantRequests {
-		t.Errorf("server got %d requests, want %d", sent, wantRequests)
-	}
-
-	assertEachEndpointRetriedThenFailed(t, logs, "status 302")
-}

internal/resolver/iterative.go

@@ -19,6 +19,7 @@ const (
 	queryTimeoutDuration = 2 * time.Second
 	maxRetries           = 2
 	maxDelegation        = 20
+	timeoutMultiplier    = 2
 
 	// maxLookupDepth is how many lookups of nameserver addresses may be
 	// under way one inside another. Looking up a nameserver's address
@@ -61,11 +62,19 @@ func checkCtx(ctx context.Context) error {
 	return nil
 }
 
-// tryExchange sends msg to addr over UDP, up to maxRetries times, until
+func (r *Resolver) exchangeWithTimeout(
-// a reply comes. Every try waits the same queryTimeoutDuration, or less
+	ctx context.Context,
-// when ctx's deadline comes first. When ctx has ended before a further
+	msg *dns.Msg,
-// try, it returns the error of the try already made, such as a timeout,
+	addr string,
-// or ErrContextCanceled when none was made.
+	attempt int,
+) (*dns.Msg, error) {
+	_ = attempt // timeout escalation handled by client config
+
+	resp, _, err := r.client.ExchangeContext(ctx, msg, addr)
+
+	return resp, err
+}
+
 func (r *Resolver) tryExchange(
 	ctx context.Context,
 	msg *dns.Msg,
@@ -75,16 +84,14 @@ func (r *Resolver) tryExchange(
 
 	var err error
 
-	for range maxRetries {
+	for attempt := range maxRetries {
 		if checkCtx(ctx) != nil {
-			if err == nil {
+			return nil, ErrContextCanceled
-				return nil, ErrContextCanceled
-			}
-
-			return nil, err
 		}
 
-		resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
+		resp, err = r.exchangeWithTimeout(
+			ctx, msg, addr, attempt,
+		)
 		if err == nil {
 			break
 		}
@@ -870,10 +877,6 @@ func (r *Resolver) querySingleType(
 			if err != nil {
 				err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
 			}
-		case errors.Is(err, ErrContextCanceled):
-			// The caller's context ended before the query was sent.
-			// That says nothing about the nameserver, so no reason is
-			// recorded.
 		case isTimeout(err):
 			state.gotTimeout = true
 		case errors.Is(err, ErrRefused):

internal/resolver/iterative_internal_test.go

@@ -1,8 +1,6 @@
 package resolver
 
 import (
-	"context"
-	"log/slog"
 	"strconv"
 	"syscall"
 	"testing"
@@ -134,33 +132,3 @@ func TestReadReply(t *testing.T) {
 		})
 	}
 }
-
-// TestQuerySingleType_ContextEnded asks a nameserver about one record
-// type with a context that has already ended, so no query is sent. A
-// query the context ended before it was sent says nothing about the
-// nameserver and records no reason: a nameserver whose earlier query
-// timed out is still reported as timed out, not as a network error.
-func TestQuerySingleType_ContextEnded(t *testing.T) {
-	t.Parallel()
-
-	r := NewFromLogger(slog.New(slog.DiscardHandler))
-
-	ctx, cancel := context.WithCancel(t.Context())
-	cancel()
-
-	resp := &NameserverResponse{Records: map[string][]string{}, Status: StatusOK}
-
-	// The query for an earlier record type timed out.
-	state := queryState{gotTimeout: true}
-
-	_, err := r.querySingleType(
-		ctx, []string{"192.0.2.1"}, "example.com.", dns.TypeA, resp, &state,
-	)
-	require.ErrorIs(t, err, ErrContextCanceled)
-	assert.Equal(t, queryState{gotTimeout: true}, state)
-
-	classifyResponse(resp, state)
-
-	assert.Equal(t, StatusTimeout, resp.Status)
-	assert.Equal(t, "all queries timed out", resp.Error)
-}

internal/resolver/resolver_test.go

@@ -1380,10 +1380,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
 	r := newTestResolver(t)
 
 	// Nothing answers at 192.0.2.1, a documentation address. The
-	// resolver tries each query twice, and each try gives up after two
+	// resolver tries each query twice, and the first try gives up
-	// seconds. This deadline ends during the second try of the first
+	// after two seconds. A deadline that ends during the first try
-	// record type; TestQueryNameserverIP_DeadlineDuringFirstTry covers
+	// makes the status vary from run to run between error and
-	// one that ends during the first.
+	// timeout, so the deadline must outlast the first try.
 	ctx, cancel := context.WithTimeout(
 		context.Background(), 3*time.Second,
 	)
@@ -1399,62 +1399,6 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
 	assert.NotEmpty(t, resp.Error)
 }
 
-// TestQueryNameserverIP_DeadlineDuringFirstTry gives a query to
-// 192.0.2.1, where nothing answers, a deadline that ends during its
-// first try. That try ends at the deadline as a timeout, so the
-// nameserver is reported as timed out, not as answering.
-func TestQueryNameserverIP_DeadlineDuringFirstTry(t *testing.T) {
-	t.Parallel()
-
-	r := newTestResolver(t)
-
-	// Well under the two seconds one try waits.
-	ctx, cancel := context.WithTimeout(
-		context.Background(), 500*time.Millisecond,
-	)
-	t.Cleanup(cancel)
-
-	resp, err := r.QueryNameserverIP(
-		ctx, "unreachable.test.", "192.0.2.1",
-		"example.com",
-	)
-	require.NoError(t, err)
-
-	assert.Equal(t, resolver.StatusTimeout, resp.Status)
-	assert.NotEmpty(t, resp.Error)
-}
-
-// TestQueryNameserverIP_CancelDuringFirstTry cancels the context while
-// the first try of a query to 192.0.2.1, where nothing answers, waits
-// for a reply. That try's timeout is all that is learned about the
-// nameserver, so it is reported as timed out, not as answering.
-func TestQueryNameserverIP_CancelDuringFirstTry(t *testing.T) {
-	t.Parallel()
-
-	r := newTestResolver(t)
-
-	// The first try starts at once and waits two seconds for a reply;
-	// the cancel comes a second in. The DNS client cuts a try short at
-	// the context's deadline but not at a cancel, so the try still
-	// waits its full two seconds and ends as a timeout a second after
-	// the cancel. By then the context has certainly ended, so no second
-	// try is made. With a deadline instead, the try ends at the moment
-	// the context does, and whether a second try is made depends on
-	// which the resolver sees first.
-	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
-	time.AfterFunc(time.Second, cancel)
-
-	resp, err := r.QueryNameserverIP(
-		ctx, "unreachable.test.", "192.0.2.1",
-		"example.com",
-	)
-	require.NoError(t, err)
-
-	assert.Equal(t, resolver.StatusTimeout, resp.Status)
-	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

internal/server/routes.go

@@ -8,6 +8,8 @@ import (
 	"github.com/go-chi/chi/v5"
 	chimw "github.com/go-chi/chi/v5/middleware"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+
+	"sneak.berlin/go/dnswatcher/static"
 )
 
 // requestTimeout is the maximum duration for handling a request.
@@ -45,7 +47,13 @@ func (s *Server) SetupRoutes() {
 	public.Get("/", s.handlers.HandleDashboard())
 
 	// Static assets (embedded CSS/JS)
-	public.Mount("/s", http.StripPrefix("/s", s.handlers.HandleStatic()))
+	public.Mount(
+		"/s",
+		http.StripPrefix(
+			"/s",
+			http.FileServer(http.FS(static.Static)),
+		),
+	)
 
 	// Health check (standard well-known path)
 	public.Get(

internal/server/routes_test.go

@@ -1,7 +1,6 @@
 package server_test
 
 import (
-	"mime"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -289,42 +288,3 @@ func TestMetricsRateLimitComesBeforeAuth(t *testing.T) {
 		t.Errorf("another address: status = %d, want 200", rec.Code)
 	}
 }
-
-// TestStaticDirectoriesAnswerNotFound checks that a request naming a
-// directory of the static assets, with or without the trailing slash,
-// answers 404 rather than a listing of it or a redirect to one, and
-// that the stylesheet is still served as CSS.
-func TestStaticDirectoriesAnswerNotFound(t *testing.T) {
-	viper.Reset()
-	t.Setenv("DNSWATCHER_TARGETS", "example.com")
-
-	srv := routedServer(t)
-
-	for _, path := range []string{"/s", "/s/", "/s/css", "/s/css/"} {
-		rec := serve(srv, httptest.NewRequestWithContext(
-			t.Context(), http.MethodGet, path, nil,
-		))
-
-		if rec.Code != http.StatusNotFound {
-			t.Errorf("GET %s: status = %d, want 404", path, rec.Code)
-		}
-	}
-
-	rec := serve(srv, httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/s/css/tailwind.min.css", nil,
-	))
-
-	if rec.Code != http.StatusOK {
-		t.Errorf("GET stylesheet: status = %d, want 200", rec.Code)
-	}
-
-	contentType := rec.Header().Get("Content-Type")
-
-	mediaType, _, err := mime.ParseMediaType(contentType)
-	if err != nil || mediaType != "text/css" {
-		t.Errorf(
-			"GET stylesheet: Content-Type = %q, want text/css",
-			contentType,
-		)
-	}
-}