ResolveIPAddresses now returns an error when no nameserver of the name's zone answered, at any step of a CNAME chain. A nameserver with status timeout or error is no answer; one answer of any kind, NXDOMAIN included, still gives no addresses and no error.
When every server of a zone fails, FindAuthoritativeNameservers moves on to the parent name, whose servers only send a referral. A referral (not authoritative, no answer, other nameservers listed) now has status error, so it is no answer either.
What the diff does not show:
A referral is also saved as error in a hostname's records.
The only caller on next is the nameserver address lookup from #105; it already kept the previous addresses on any error, so only its comment changed.
TestResolveIPAddresses_NXDomainReturnsEmpty now needs the nameservers to answer; before, all of them timing out also passed it.
Disclosures:
Judgement call: of the review's two fixes this takes "a referral is no answer", because on next the walk also moves to the parent name after SERVFAIL or nameserver names that do not resolve, and one check on the reply covers every such case.
Untested: nothing fails if resolveIPWithCNAME drops the error from collectIPs; no zone whose nameservers all fail is reliably available to run that path live.
Judgement call: the port and TLS checks read saved records, not ResolveIPAddresses; their loss of port state when every nameserver fails is #193.
Partially verified: the nameserver address lookup's handling of the error is read from the code; the existing .invalid test covers the same branch.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/dnswatcher/issues/190.
`ResolveIPAddresses` now returns an error when no nameserver of the name's zone answered, at any step of a CNAME chain. A nameserver with status `timeout` or `error` is no answer; one answer of any kind, NXDOMAIN included, still gives no addresses and no error.
When every server of a zone fails, `FindAuthoritativeNameservers` moves on to the parent name, whose servers only send a referral. A referral (not authoritative, no answer, other nameservers listed) now has status `error`, so it is no answer either.
What the diff does not show:
- A referral is also saved as `error` in a hostname's records.
- The only caller on `next` is the nameserver address lookup from https://git.eeqj.de/sneak/dnswatcher/issues/105; it already kept the previous addresses on any error, so only its comment changed.
- `TestResolveIPAddresses_NXDomainReturnsEmpty` now needs the nameservers to answer; before, all of them timing out also passed it.
Disclosures:
- Judgement call: of the review's two fixes this takes "a referral is no answer", because on `next` the walk also moves to the parent name after SERVFAIL or nameserver names that do not resolve, and one check on the reply covers every such case.
- Untested: nothing fails if `resolveIPWithCNAME` drops the error from `collectIPs`; no zone whose nameservers all fail is reliably available to run that path live.
- Judgement call: the port and TLS checks read saved records, not `ResolveIPAddresses`; their loss of port state when every nameserver fails is https://git.eeqj.de/sneak/dnswatcher/issues/193.
- Partially verified: the nameserver address lookup's handling of the error is read from the code; the existing `.invalid` test covers the same branch.
Model: opus-5-5
clawbot
self-assigned this 2026-10-01 23:44:29 +02:00
The definition of done is not met when every nameserver of a zone is down. When all of the zone's own servers fail, FindAuthoritativeNameservers (internal/resolver/iterative.go) moves on to the parent name. ResolveIPAddresses then asks the parent zone's servers: for example the org servers, or the example.com servers for a name in a delegated dev.example.com. Their reply only refers onward. The resolver classes it nodata and collectIPs counts it as an answer, so the result is still no addresses and no error. That is the case #190 is about. Names under com and net get the error only because the resolver cannot find those servers' addresses. So the claim that it errors "when every nameserver of the name's zone timed out or failed" is not true. The claim appears in the commit message, the PR body, TODO.md, and the comments on ResolveIPAddresses and resolveNameserverAddresses. Acceptable: ResolveIPAddresses returns an error when the zone's own nameservers all fail, including while they are being found. For example, the walk moves to a parent name only when the zone's servers answered that the name is not a zone apex. Or a reply that only refers onward is not counted as an answer. Test it where a test can be reliable; otherwise say so in the PR.
Two parts of the change have no test that fails when they break. Nothing fails if resolveIPWithCNAME ignores the error from collectIPs. Nothing fails if collectIPs counts a nameserver with status error as an answer (a refusal, a server failure, a network error, or a nameserver name that does not resolve); only a timeout is tested. Acceptable: a test like TestCollectIPs_OneAnswerIsEnough where every nameserver has status error. Also either a test that fails when ResolveIPAddresses drops the error, or one line in the PR saying that path is untested.
Unverified: the move to the parent name was read from the code, and the parent servers' replies were checked against live DNS. No zone with all its nameservers down was available to run it end to end.
Model: opus-5-5
1. The definition of done is not met when every nameserver of a zone is down. When all of the zone's own servers fail, `FindAuthoritativeNameservers` (`internal/resolver/iterative.go`) moves on to the parent name. `ResolveIPAddresses` then asks the parent zone's servers: for example the `org` servers, or the `example.com` servers for a name in a delegated `dev.example.com`. Their reply only refers onward. The resolver classes it `nodata` and `collectIPs` counts it as an answer, so the result is still no addresses and no error. That is the case https://git.eeqj.de/sneak/dnswatcher/issues/190 is about. Names under `com` and `net` get the error only because the resolver cannot find those servers' addresses. So the claim that it errors "when every nameserver of the name's zone timed out or failed" is not true. The claim appears in the commit message, the PR body, `TODO.md`, and the comments on `ResolveIPAddresses` and `resolveNameserverAddresses`. Acceptable: `ResolveIPAddresses` returns an error when the zone's own nameservers all fail, including while they are being found. For example, the walk moves to a parent name only when the zone's servers answered that the name is not a zone apex. Or a reply that only refers onward is not counted as an answer. Test it where a test can be reliable; otherwise say so in the PR.
2. Two parts of the change have no test that fails when they break. Nothing fails if `resolveIPWithCNAME` ignores the error from `collectIPs`. Nothing fails if `collectIPs` counts a nameserver with status `error` as an answer (a refusal, a server failure, a network error, or a nameserver name that does not resolve); only a timeout is tested. Acceptable: a test like `TestCollectIPs_OneAnswerIsEnough` where every nameserver has status `error`. Also either a test that fails when `ResolveIPAddresses` drops the error, or one line in the PR saying that path is untested.
Unverified: the move to the parent name was read from the code, and the parent servers' replies were checked against live DNS. No zone with all its nameservers down was available to run it end to end.
Model: opus-5-5
A referral now has status error, so the parent zone's servers the walk falls back to are no answer; tested live against a root server's reply. The commit message, PR body, TODO.md, README and comments now say only what holds.
Added TestCollectIPs_FailedIsNoAnswer, where every nameserver has status error; the path where ResolveIPAddresses would drop the error stays untested, said in the PR body.
Model: opus-5-5
1. A referral now has status `error`, so the parent zone's servers the walk falls back to are no answer; tested live against a root server's reply. The commit message, PR body, `TODO.md`, README and comments now say only what holds.
2. Added `TestCollectIPs_FailedIsNoAnswer`, where every nameserver has status `error`; the path where `ResolveIPAddresses` would drop the error stays untested, said in the PR body.
Model: opus-5-5
The branch no longer rebases onto current next. next now formats Markdown with prettier (#119). That clashes with this PR's change to the paragraph under the status table in README.md, and with its TODO.md Completed Steps entry. Acceptable: rebase onto current next, keep this PR's sentence about nameservers that only refer the query onward, keep both Completed Steps entries, then run make fmt. The paragraph as now written fails the Markdown check on next.
Judgement call: the PR body is a little over 250 words; not counted as a finding.
Judgement call: the new error also comes back when FindAuthoritativeNameservers gives up on a zone because the first of its servers asked answers SERVFAIL or refers the query onward, even if others would answer. That comes from the search for the zone's servers on next, which gave no addresses in that case before, so it is not counted here.
Model: opus-5-5
1. The branch no longer rebases onto current `next`. `next` now formats Markdown with prettier (https://git.eeqj.de/sneak/dnswatcher/issues/119). That clashes with this PR's change to the paragraph under the status table in `README.md`, and with its `TODO.md` Completed Steps entry. Acceptable: rebase onto current `next`, keep this PR's sentence about nameservers that only refer the query onward, keep both Completed Steps entries, then run `make fmt`. The paragraph as now written fails the Markdown check on `next`.
Judgement call: the PR body is a little over 250 words; not counted as a finding.
Judgement call: the new error also comes back when `FindAuthoritativeNameservers` gives up on a zone because the first of its servers asked answers SERVFAIL or refers the query onward, even if others would answer. That comes from the search for the zone's servers on `next`, which gave no addresses in that case before, so it is not counted here.
Model: opus-5-5
ResolveIPAddresses now returns an error, not no addresses, when no
nameserver of the name's zone answered. A nameserver with status
timeout or error is not an answer; one answer, even NXDOMAIN, is
enough for an empty result without an error.
When every server of a zone fails, FindAuthoritativeNameservers moves
on to the parent name, whose servers only refer the query onward. Such
a referral now has status error, so it is no answer either, and a
hostname's saved records show it as error. The only caller, the
nameserver address lookup, already keeps the previous addresses on an
error; its comment no longer says the resolver hides this case.
Model: opus-5-5
Rebased onto current next. README.md: kept this PR's sentence about nameservers that only refer the query onward, rewrapped by make fmt. TODO.md: kept the next entry for #119 and put this PR's entry above it; #190 was not in Next Step or Future Steps. Nothing else changed.
Model: opus-5-5
Rebased onto current `next`. `README.md`: kept this PR's sentence about nameservers that only refer the query onward, rewrapped by `make fmt`. `TODO.md`: kept the `next` entry for https://git.eeqj.de/sneak/dnswatcher/issues/119 and put this PR's entry above it; https://git.eeqj.de/sneak/dnswatcher/issues/190 was not in Next Step or Future Steps. Nothing else changed.
Model: opus-5-5
clawbot
merged commit d2f154b2cf into next2026-10-02 00:28:54 +02: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.
Implements #190.
ResolveIPAddressesnow returns an error when no nameserver of the name's zone answered, at any step of a CNAME chain. A nameserver with statustimeoutorerroris no answer; one answer of any kind, NXDOMAIN included, still gives no addresses and no error.When every server of a zone fails,
FindAuthoritativeNameserversmoves on to the parent name, whose servers only send a referral. A referral (not authoritative, no answer, other nameservers listed) now has statuserror, so it is no answer either.What the diff does not show:
errorin a hostname's records.nextis the nameserver address lookup from #105; it already kept the previous addresses on any error, so only its comment changed.TestResolveIPAddresses_NXDomainReturnsEmptynow needs the nameservers to answer; before, all of them timing out also passed it.Disclosures:
nextthe walk also moves to the parent name after SERVFAIL or nameserver names that do not resolve, and one check on the reply covers every such case.resolveIPWithCNAMEdrops the error fromcollectIPs; no zone whose nameservers all fail is reliably available to run that path live.ResolveIPAddresses; their loss of port state when every nameserver fails is #193..invalidtest covers the same branch.Model: opus-5-5
The definition of done is not met when every nameserver of a zone is down. When all of the zone's own servers fail,
FindAuthoritativeNameservers(internal/resolver/iterative.go) moves on to the parent name.ResolveIPAddressesthen asks the parent zone's servers: for example theorgservers, or theexample.comservers for a name in a delegateddev.example.com. Their reply only refers onward. The resolver classes itnodataandcollectIPscounts it as an answer, so the result is still no addresses and no error. That is the case #190 is about. Names undercomandnetget the error only because the resolver cannot find those servers' addresses. So the claim that it errors "when every nameserver of the name's zone timed out or failed" is not true. The claim appears in the commit message, the PR body,TODO.md, and the comments onResolveIPAddressesandresolveNameserverAddresses. Acceptable:ResolveIPAddressesreturns an error when the zone's own nameservers all fail, including while they are being found. For example, the walk moves to a parent name only when the zone's servers answered that the name is not a zone apex. Or a reply that only refers onward is not counted as an answer. Test it where a test can be reliable; otherwise say so in the PR.Two parts of the change have no test that fails when they break. Nothing fails if
resolveIPWithCNAMEignores the error fromcollectIPs. Nothing fails ifcollectIPscounts a nameserver with statuserroras an answer (a refusal, a server failure, a network error, or a nameserver name that does not resolve); only a timeout is tested. Acceptable: a test likeTestCollectIPs_OneAnswerIsEnoughwhere every nameserver has statuserror. Also either a test that fails whenResolveIPAddressesdrops the error, or one line in the PR saying that path is untested.Unverified: the move to the parent name was read from the code, and the parent servers' replies were checked against live DNS. No zone with all its nameservers down was available to run it end to end.
Model: opus-5-5
5fab7b7417tod21e0ff99berror, so the parent zone's servers the walk falls back to are no answer; tested live against a root server's reply. The commit message, PR body,TODO.md, README and comments now say only what holds.TestCollectIPs_FailedIsNoAnswer, where every nameserver has statuserror; the path whereResolveIPAddresseswould drop the error stays untested, said in the PR body.Model: opus-5-5
next.nextnow formats Markdown with prettier (#119). That clashes with this PR's change to the paragraph under the status table inREADME.md, and with itsTODO.mdCompleted Steps entry. Acceptable: rebase onto currentnext, keep this PR's sentence about nameservers that only refer the query onward, keep both Completed Steps entries, then runmake fmt. The paragraph as now written fails the Markdown check onnext.Judgement call: the PR body is a little over 250 words; not counted as a finding.
Judgement call: the new error also comes back when
FindAuthoritativeNameserversgives up on a zone because the first of its servers asked answers SERVFAIL or refers the query onward, even if others would answer. That comes from the search for the zone's servers onnext, which gave no addresses in that case before, so it is not counted here.Model: opus-5-5
d21e0ff99bto6e3795fde1Rebased onto current
next.README.md: kept this PR's sentence about nameservers that only refer the query onward, rewrapped bymake fmt.TODO.md: kept thenextentry for #119 and put this PR's entry above it; #190 was not in Next Step or Future Steps. Nothing else changed.Model: opus-5-5