tryExchange discards a prior attempt's timeout error on context cancellation, downgrading StatusTimeout to StatusNoData #141

New Issue

2026-09-04T01:22:19+02:00

clawbot commented

2026-09-04 01:22:19 +02:00

Found while restoring transport-failure test coverage in PR #97. Deliberately not fixed there — it is a behaviour change outside that PR's mandate.

Problem

tryExchange re-checks the context at the top of each retry iteration and returns ErrContextCanceled, throwing away the error recorded by the previous attempt. When a caller's deadline expires mid-query, the genuine timeout from attempt N is replaced by a context-cancellation error on attempt N+1, and the result classifies as StatusNoData instead of StatusTimeout.

Impact

Reachable in production: the watcher passes deadline-bounded contexts. A nameserver that is genuinely timing out can be reported as "no data" whenever the caller deadline happens to expire during the retry loop, which is the common case for a slow/blackholed server.

Expected

If a previous attempt already produced a timeout, that classification should survive context cancellation rather than being overwritten.

Notes

The transport tests added in PR #97 use a generous 30s context specifically so they do not depend on this buggy path. A fix should add coverage that exercises a short caller deadline against a silent responder and asserts StatusTimeout.
Related but distinct from #120 (server timeout tests protect the constructor but not the call site).

Found while restoring transport-failure test coverage in [PR #97](https://git.eeqj.de/sneak/dnswatcher/pulls/97). Deliberately **not** fixed there — it is a behaviour change outside that PR's mandate. ## Problem `tryExchange` re-checks the context at the top of each retry iteration and returns `ErrContextCanceled`, throwing away the error recorded by the previous attempt. When a caller's deadline expires mid-query, the genuine timeout from attempt N is replaced by a context-cancellation error on attempt N+1, and the result classifies as `StatusNoData` instead of `StatusTimeout`. ## Impact Reachable in production: the watcher passes deadline-bounded contexts. A nameserver that is genuinely timing out can be reported as "no data" whenever the caller deadline happens to expire during the retry loop, which is the common case for a slow/blackholed server. ## Expected If a previous attempt already produced a timeout, that classification should survive context cancellation rather than being overwritten. ## Notes - The transport tests added in PR #97 use a generous 30s context specifically so they do not depend on this buggy path. A fix should add coverage that exercises a short caller deadline against a silent responder and asserts `StatusTimeout`. - Related but distinct from [#120](https://git.eeqj.de/sneak/dnswatcher/issues/120) (server timeout tests protect the constructor but not the call site).

~~clawbot referenced this issue 2026-09-04 02:19:35 +02:00~~

Remove DNS mocking from tests #97

clawbot referenced this issue

2026-09-04 02:23:38 +02:00

Live-DNS test harness never retries an answer that is missing the record type under test, so TestQueryNameserver_TXT flakes #143

clawbot commented

2026-09-05 11:32:51 +02:00

Closed at sneak's instruction, 2026-09-05: this was opened by an agent running on another machine outside the managed fleet, under superseded rules. Reopen if the content is wanted.

Model: fable-5-1

Closed at sneak's instruction, 2026-09-05: this was opened by an agent running on another machine outside the managed fleet, under superseded rules. Reopen if the content is wanted. Model: fable-5-1

clawbot closed this issue

2026-09-05 11:32:51 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/dnswatcher#141