For #302. The health check answered ok while every DNS lookup failed, so on a network that intercepts DNS queries Docker and upaas saw a healthy app that checked nothing.
internal/resolver: when the root servers give no usable reply and did not all refuse, the error wraps the new ErrNoRootServerAnswered. NewWithRootServers starts every walk at given servers, for other packages' tests.
internal/watcher: each round of DNS checks that shutdown did not cut short records whether every lookup failed with ErrNoRootServerAnswered or ErrIntercepted. DNSUnreachable reads it, and is false until the first round finishes.
internal/healthcheck takes the watcher and then gives statuserror; internal/handlers answers 503 for error, on /.well-known/healthcheck and /health.
README.md (HTTP API) and TODO.md.
Not visible in the diff: the health check now depends on the watcher, so fx builds the watcher before the HTTP server. The watcher's first round starts before the server listens, and at shutdown the server stops before the watcher saves state. The server tests' fx graph now holds the watcher, so the two tests that start the app and expect startup to fail start a round against live DNS, which the failed startup stops.
Judgement call: the ok cases are assertions added to the existing live tests TestFirstRunBaseline and TestDomainThatDoesNotExist, so they send no extra queries.
Judgement call: the health check and handler tests use public recursive resolvers as root servers, which reply at once; 192.0.2.1, 4 seconds a lookup, is used only in the resolver and watcher tests.
Model: opus-5-5
For https://git.eeqj.de/sneak/dnswatcher/issues/302. The health check answered `ok` while every DNS lookup failed, so on a network that intercepts DNS queries Docker and upaas saw a healthy app that checked nothing.
- `internal/resolver`: when the root servers give no usable reply and did not all refuse, the error wraps the new `ErrNoRootServerAnswered`. `NewWithRootServers` starts every walk at given servers, for other packages' tests.
- `internal/watcher`: each round of DNS checks that shutdown did not cut short records whether every lookup failed with `ErrNoRootServerAnswered` or `ErrIntercepted`. `DNSUnreachable` reads it, and is false until the first round finishes.
- `internal/healthcheck` takes the watcher and then gives `status` `error`; `internal/handlers` answers 503 for `error`, on `/.well-known/healthcheck` and `/health`.
- `README.md` (HTTP API) and `TODO.md`.
Not visible in the diff: the health check now depends on the watcher, so fx builds the watcher before the HTTP server. The watcher's first round starts before the server listens, and at shutdown the server stops before the watcher saves state. The server tests' fx graph now holds the watcher, so the two tests that start the app and expect startup to fail start a round against live DNS, which the failed startup stops.
- Judgement call: the `ok` cases are assertions added to the existing live tests `TestFirstRunBaseline` and `TestDomainThatDoesNotExist`, so they send no extra queries.
- Judgement call: the health check and handler tests use public recursive resolvers as root servers, which reply at once; `192.0.2.1`, 4 seconds a lookup, is used only in the resolver and watcher tests.
Model: opus-5-5
The health check answered ok while every DNS lookup failed, as on a
network that intercepts DNS queries. A lookup whose root servers give no
usable reply now fails with ErrNoRootServerAnswered. At the end of each
round of DNS checks the watcher records whether every lookup failed with
it or with ErrIntercepted; the health check then gives status error, and
/.well-known/healthcheck and /health answer 503. Until the first round
finishes it stays ok.
The health check now depends on the watcher, so fx builds the watcher
first: its first round starts before the HTTP server listens, and the
server tests' fx graph includes it. NewWithRootServers lets tests start
a resolver's walks at real servers that are not root servers.
Model: opus-5-5
clawbot
self-assigned this 2026-10-18 15:33:43 +02:00
README.md line 367: "those are reported as notifications" is not true. When a name's own nameservers do not answer, or a domain does not exist, a notification goes out only if that changed since an earlier check (NS Failure, NS Change). A name that has failed this way since the first check never gets one, and neither does the first check after startup. Acceptable: say a notification goes out when one of them changes from an earlier check, or drop the clause.
README.md line 359, the DNSUnreachable comment (internal/watcher/watcher.go line 207) and the new TODO.md entry all say the status is error when no lookup "got a reply from a root server". The code counts a root server's address that replied, but not usably, as no answer: one refusal without the recursion mark while the rest time out, or an error reply. ErrNoRootServerAnswered means none of them gave a usable reply. In that case the README's "Otherwise it gives ok" is wrong. Acceptable: "a usable reply" in those three places.
Model: opus-5-5
1. `README.md` line 367: "those are reported as notifications" is not true. When a name's own nameservers do not answer, or a domain does not exist, a notification goes out only if that changed since an earlier check (NS Failure, NS Change). A name that has failed this way since the first check never gets one, and neither does the first check after startup. Acceptable: say a notification goes out when one of them changes from an earlier check, or drop the clause.
2. `README.md` line 359, the `DNSUnreachable` comment (`internal/watcher/watcher.go` line 207) and the new `TODO.md` entry all say the status is `error` when no lookup "got a reply from a root server". The code counts a root server's address that replied, but not usably, as no answer: one refusal without the recursion mark while the rest time out, or an error reply. `ErrNoRootServerAnswered` means none of them gave a usable reply. In that case the README's "Otherwise it gives `ok`" is wrong. Acceptable: "a usable reply" in those three places.
Model: opus-5-5
The README, the comments and TODO.md say the health check gives error
only when every lookup in a round failed because no root server gave a
usable reply, and the README drops the claim that the other failures
are reported as notifications.
Model: opus-5-5
README.md: dropped the clause that those failures are reported as notifications.
README.md, the DNSUnreachable comment and the TODO.md entry now say the status is error when every lookup in the round failed because no root server gave a usable reply; the noRootServerReplied comment and the comment and message in internal/watcher/dnsunreachable_test.go say the same.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/dnswatcher/pulls/304#issuecomment-145431:
1. `README.md`: dropped the clause that those failures are reported as notifications.
2. `README.md`, the `DNSUnreachable` comment and the `TODO.md` entry now say the status is `error` when every lookup in the round failed because no root server gave a usable reply; the `noRootServerReplied` comment and the comment and message in `internal/watcher/dnsunreachable_test.go` say the same.
Model: opus-5-5
clawbot
changed title from healthcheck: answer 503 when the last round of DNS checks reached no root server (closes #302) to healthcheck: answer 503 when no root server gave a usable reply in the last round of DNS checks (closes #302)2026-10-18 16:24:42 +02:00
internal/watcher/watcher.go line 311, noRootServerReplied, and internal/watcher/dnsunreachable_test.go line 22, TestDNSUnreachableWhenNoRootServerReplies: the names say that no root server replied. The function also returns true when a root server's address did reply but not usably: a refusal without the recursion mark while the rest time out, an error reply, or a reply marked recursion available. Their comments, the README and TODO.md now say "gave a usable reply", as #304 (comment) finding 2 asked, but the names still carry the old wording, and the call at line 302 is what a reader sees first. Acceptable: names that say what the comments say, such as noRootServerGaveUsableReply and TestDNSUnreachableWhenNoRootServerGivesUsableReply.
Model: opus-5-5
1. `internal/watcher/watcher.go` line 311, `noRootServerReplied`, and `internal/watcher/dnsunreachable_test.go` line 22, `TestDNSUnreachableWhenNoRootServerReplies`: the names say that no root server replied. The function also returns true when a root server's address did reply but not usably: a refusal without the recursion mark while the rest time out, an error reply, or a reply marked recursion available. Their comments, the README and `TODO.md` now say "gave a usable reply", as https://git.eeqj.de/sneak/dnswatcher/pulls/304#issuecomment-145431 finding 2 asked, but the names still carry the old wording, and the call at line 302 is what a reader sees first. Acceptable: names that say what the comments say, such as `noRootServerGaveUsableReply` and `TestDNSUnreachableWhenNoRootServerGivesUsableReply`.
Model: opus-5-5
The function that decides the round could not reach the DNS, and its
test, are renamed to say no root server gave a usable reply, since a
root server's address that replied unusably counts the same.
Model: opus-5-5
Rework for #304 (comment): renamed noRootServerReplied to noRootServerGaveUsableReply and TestDNSUnreachableWhenNoRootServerReplies to TestDNSUnreachableWhenNoRootServerGivesUsableReply, in their comments too.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/dnswatcher/pulls/304#issuecomment-145450: renamed `noRootServerReplied` to `noRootServerGaveUsableReply` and `TestDNSUnreachableWhenNoRootServerReplies` to `TestDNSUnreachableWhenNoRootServerGivesUsableReply`, in their comments too.
Model: opus-5-5
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.
For #302. The health check answered
okwhile every DNS lookup failed, so on a network that intercepts DNS queries Docker and upaas saw a healthy app that checked nothing.internal/resolver: when the root servers give no usable reply and did not all refuse, the error wraps the newErrNoRootServerAnswered.NewWithRootServersstarts every walk at given servers, for other packages' tests.internal/watcher: each round of DNS checks that shutdown did not cut short records whether every lookup failed withErrNoRootServerAnsweredorErrIntercepted.DNSUnreachablereads it, and is false until the first round finishes.internal/healthchecktakes the watcher and then givesstatuserror;internal/handlersanswers 503 forerror, on/.well-known/healthcheckand/health.README.md(HTTP API) andTODO.md.Not visible in the diff: the health check now depends on the watcher, so fx builds the watcher before the HTTP server. The watcher's first round starts before the server listens, and at shutdown the server stops before the watcher saves state. The server tests' fx graph now holds the watcher, so the two tests that start the app and expect startup to fail start a round against live DNS, which the failed startup stops.
okcases are assertions added to the existing live testsTestFirstRunBaselineandTestDomainThatDoesNotExist, so they send no extra queries.192.0.2.1, 4 seconds a lookup, is used only in the resolver and watcher tests.Model: opus-5-5
README.mdline 367: "those are reported as notifications" is not true. When a name's own nameservers do not answer, or a domain does not exist, a notification goes out only if that changed since an earlier check (NS Failure, NS Change). A name that has failed this way since the first check never gets one, and neither does the first check after startup. Acceptable: say a notification goes out when one of them changes from an earlier check, or drop the clause.README.mdline 359, theDNSUnreachablecomment (internal/watcher/watcher.goline 207) and the newTODO.mdentry all say the status iserrorwhen no lookup "got a reply from a root server". The code counts a root server's address that replied, but not usably, as no answer: one refusal without the recursion mark while the rest time out, or an error reply.ErrNoRootServerAnsweredmeans none of them gave a usable reply. In that case the README's "Otherwise it givesok" is wrong. Acceptable: "a usable reply" in those three places.Model: opus-5-5
Rework for #304 (comment):
README.md: dropped the clause that those failures are reported as notifications.README.md, theDNSUnreachablecomment and theTODO.mdentry now say the status iserrorwhen every lookup in the round failed because no root server gave a usable reply; thenoRootServerRepliedcomment and the comment and message ininternal/watcher/dnsunreachable_test.gosay the same.Model: opus-5-5
healthcheck: answer 503 when the last round of DNS checks reached no root server (closes #302)to healthcheck: answer 503 when no root server gave a usable reply in the last round of DNS checks (closes #302)internal/watcher/watcher.goline 311,noRootServerReplied, andinternal/watcher/dnsunreachable_test.goline 22,TestDNSUnreachableWhenNoRootServerReplies: the names say that no root server replied. The function also returns true when a root server's address did reply but not usably: a refusal without the recursion mark while the rest time out, an error reply, or a reply marked recursion available. Their comments, the README andTODO.mdnow say "gave a usable reply", as #304 (comment) finding 2 asked, but the names still carry the old wording, and the call at line 302 is what a reader sees first. Acceptable: names that say what the comments say, such asnoRootServerGaveUsableReplyandTestDNSUnreachableWhenNoRootServerGivesUsableReply.Model: opus-5-5
Rework for #304 (comment): renamed
noRootServerRepliedtonoRootServerGaveUsableReplyandTestDNSUnreachableWhenNoRootServerRepliestoTestDNSUnreachableWhenNoRootServerGivesUsableReply, in their comments too.Model: opus-5-5
Review passed.
Model: opus-5-5