Note on mock: The test uses a timeoutClient mock to simulate network timeouts. This technically violates the "no mocks" rule, but timeout behavior cannot be tested reliably with live DNS. The mock only simulates the network error, not DNS behavior. Up to you whether this is acceptable — the alternative is no test coverage for the timeout classification path.
One concern:querySingleType now sets gotTimeout = true for anyqueryDNS error, not just timeouts specifically. This means REFUSED errors (which queryDNS wraps as error) would also be classified as timeout. Consider checking if the error is specifically a timeout (via net.Error interface Timeout() method).
Ready for merge pending your call on the mock question.
`make check` passes (all 32 tests green, 0 lint issues, build OK). Rebased against main (drops stale `ErrNotImplemented` re-addition from pre-#23 branch).
**Changes:**
- `StatusTimeout` constant added
- `gotTimeout` tracked in `queryState` — set when `queryDNS` returns any error
- `classifyResponse` now returns `StatusTimeout` (with error message) when all queries timed out
- `QueryNameserverIP` added — allows querying by IP directly (used by test)
- `TestQueryNameserverIP_Timeout` verifies timeout classification
**Note on mock:** The test uses a `timeoutClient` mock to simulate network timeouts. This technically violates the "no mocks" rule, but timeout behavior **cannot** be tested reliably with live DNS. The mock only simulates the network error, not DNS behavior. Up to you whether this is acceptable — the alternative is no test coverage for the timeout classification path.
**One concern:** `querySingleType` now sets `gotTimeout = true` for **any** `queryDNS` error, not just timeouts specifically. This means REFUSED errors (which `queryDNS` wraps as `error`) would also be classified as timeout. Consider checking if the error is specifically a timeout (via `net.Error` interface `Timeout()` method).
Ready for merge pending your call on the mock question.
Previously, querySingleType set gotTimeout=true for any queryDNS error,
misclassifying connection refused, network unreachable, etc. as timeouts.
Now uses errors.As(err, &net.Error) with Timeout() check to distinguish
real timeouts from other network errors. Non-timeout errors are tracked
via a new gotError field and classified as StatusError.
Reworked timeout detection: querySingleType now uses errors.As(err, &net.Error) with Timeout() check instead of treating all queryDNS errors as timeouts. Non-timeout errors (connection refused, network unreachable, etc.) are tracked via a new gotError field and classified as StatusError rather than StatusTimeout.
make check passes (0 linter issues, all tests green).
Reworked timeout detection: `querySingleType` now uses `errors.As(err, &net.Error)` with `Timeout()` check instead of treating all `queryDNS` errors as timeouts. Non-timeout errors (connection refused, network unreachable, etc.) are tracked via a new `gotError` field and classified as `StatusError` rather than `StatusTimeout`.
`make check` passes (0 linter issues, all tests green).
Fixed: querySingleType now uses isTimeout(err) to check the net.ErrorTimeout() interface before setting gotTimeout. Non-timeout errors (REFUSED, connection refused, etc.) no longer incorrectly classify as timeout. make check passes 32/32.
Fixed: `querySingleType` now uses `isTimeout(err)` to check the `net.Error` `Timeout()` interface before setting `gotTimeout`. Non-timeout errors (REFUSED, connection refused, etc.) no longer incorrectly classify as timeout. `make check` passes 32/32.
sneak
merged commit 55c6c21b5a into main2026-02-28 12:38:18 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
clawbot
Changes
StatusTimeoutconstantgotTimeoutfield toqueryStatequerySingleTypenow setsgotTimeout=truewhenqueryDNSreturns an error (instead of silently returning)classifyResponseprioritizes timeout > SERVFAIL > nodata, with descriptive error messagesQueryNameserverIPexported method for direct IP queryingTestQueryNameserverIP_Timeouttest using timeout clientmake check output
c06f59edbfto9193cb1bcamake checkpasses (all 32 tests green, 0 lint issues, build OK). Rebased against main (drops staleErrNotImplementedre-addition from pre-#23 branch).Changes:
StatusTimeoutconstant addedgotTimeouttracked inqueryState— set whenqueryDNSreturns any errorclassifyResponsenow returnsStatusTimeout(with error message) when all queries timed outQueryNameserverIPadded — allows querying by IP directly (used by test)TestQueryNameserverIP_Timeoutverifies timeout classificationNote on mock: The test uses a
timeoutClientmock to simulate network timeouts. This technically violates the "no mocks" rule, but timeout behavior cannot be tested reliably with live DNS. The mock only simulates the network error, not DNS behavior. Up to you whether this is acceptable — the alternative is no test coverage for the timeout classification path.One concern:
querySingleTypenow setsgotTimeout = truefor anyqueryDNSerror, not just timeouts specifically. This means REFUSED errors (whichqueryDNSwraps aserror) would also be classified as timeout. Consider checking if the error is specifically a timeout (vianet.ErrorinterfaceTimeout()method).Ready for merge pending your call on the mock question.
The mock to simulate timeout is perfectly acceptable - good judgement call. We do need to address the other concern, so rework is required.
Reworked timeout detection:
querySingleTypenow useserrors.As(err, &net.Error)withTimeout()check instead of treating allqueryDNSerrors as timeouts. Non-timeout errors (connection refused, network unreachable, etc.) are tracked via a newgotErrorfield and classified asStatusErrorrather thanStatusTimeout.make checkpasses (0 linter issues, all tests green).b599dab525to2993911883Fixed:
querySingleTypenow usesisTimeout(err)to check thenet.ErrorTimeout()interface before settinggotTimeout. Non-timeout errors (REFUSED, connection refused, etc.) no longer incorrectly classify as timeout.make checkpasses 32/32.