resolver: a query cut short by the caller's context records no reason (closes #298)
check / check (push) Waiting to run
check / check (push) Waiting to run
When the caller's context ends, the next query returns ErrContextCanceled before anything is sent. querySingleType recorded that as a network error, which outranks a timeout, so a nameserver that had only timed out could be reported as "network error: context canceled". It now records no reason for it. A try the context's deadline cuts short ends as a timeout. When the context has ended before a further try, tryExchange now returns that try's error instead of ErrContextCanceled, so such a nameserver is reported as timed out, not as answering. Model: opus-5-5
This commit was merged in pull request #299.
This commit is contained in:
@@ -19,6 +19,10 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
|
|||||||
|
|
||||||
# Completed Steps
|
# 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;
|
- 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
|
every try waits the same 2 seconds, and the unused `timeoutMultiplier` is gone
|
||||||
(closes #296).
|
(closes #296).
|
||||||
|
|||||||
@@ -62,7 +62,10 @@ func checkCtx(ctx context.Context) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// tryExchange sends msg to addr over UDP, up to maxRetries times, until
|
// tryExchange sends msg to addr over UDP, up to maxRetries times, until
|
||||||
// a reply comes. Every try waits the same queryTimeoutDuration.
|
// a reply comes. Every try waits the same queryTimeoutDuration, or less
|
||||||
|
// when ctx's deadline comes first. When ctx has ended before a further
|
||||||
|
// try, it returns the error of the try already made, such as a timeout,
|
||||||
|
// or ErrContextCanceled when none was made.
|
||||||
func (r *Resolver) tryExchange(
|
func (r *Resolver) tryExchange(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
msg *dns.Msg,
|
msg *dns.Msg,
|
||||||
@@ -74,7 +77,11 @@ func (r *Resolver) tryExchange(
|
|||||||
|
|
||||||
for range maxRetries {
|
for range maxRetries {
|
||||||
if checkCtx(ctx) != nil {
|
if checkCtx(ctx) != nil {
|
||||||
return nil, ErrContextCanceled
|
if err == nil {
|
||||||
|
return nil, ErrContextCanceled
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
|
resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
|
||||||
@@ -863,6 +870,10 @@ func (r *Resolver) querySingleType(
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
|
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):
|
case isTimeout(err):
|
||||||
state.gotTimeout = true
|
state.gotTimeout = true
|
||||||
case errors.Is(err, ErrRefused):
|
case errors.Is(err, ErrRefused):
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package resolver
|
package resolver
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
|
"log/slog"
|
||||||
"strconv"
|
"strconv"
|
||||||
"syscall"
|
"syscall"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -132,3 +134,33 @@ 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)
|
||||||
|
}
|
||||||
|
|||||||
@@ -1380,10 +1380,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
|
|||||||
r := newTestResolver(t)
|
r := newTestResolver(t)
|
||||||
|
|
||||||
// Nothing answers at 192.0.2.1, a documentation address. The
|
// Nothing answers at 192.0.2.1, a documentation address. The
|
||||||
// resolver tries each query twice, and the first try gives up
|
// resolver tries each query twice, and each try gives up after two
|
||||||
// after two seconds. A deadline that ends during the first try
|
// seconds. This deadline ends during the second try of the first
|
||||||
// makes the status vary from run to run between error and
|
// record type; TestQueryNameserverIP_DeadlineDuringFirstTry covers
|
||||||
// timeout, so the deadline must outlast the first try.
|
// one that ends during the first.
|
||||||
ctx, cancel := context.WithTimeout(
|
ctx, cancel := context.WithTimeout(
|
||||||
context.Background(), 3*time.Second,
|
context.Background(), 3*time.Second,
|
||||||
)
|
)
|
||||||
@@ -1399,6 +1399,62 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
|
|||||||
assert.NotEmpty(t, resp.Error)
|
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
|
// TestQueryNameserverIP_CancelledLogsNothing cancels the context while
|
||||||
// a query to 192.0.2.1, where nothing answers, is waiting for a reply,
|
// 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
|
// as shutdown does. The query was cut short, not failed, so nothing is
|
||||||
|
|||||||
Reference in New Issue
Block a user