The resolver lists in FailedTypes each record type whose query to a nameserver got no usable reply (none after two tries, a reply code other than NOERROR or NXDOMAIN, a referral, or a truncated reply whose TCP retry failed), and logs it unless shutdown cut it short. A nameserver that answered no type fails as before.
The watcher saves such a type in the nameserver's failedTypes, keeping the previous check's records, which are compared with its next answer but not with other nameservers' on that check. When the previous check did not know them either (the nameserver new or failing then, or the type already in unknownTypes), the type is also in unknownTypes and not compared until it answers. Change messages leave out what was not compared.
Not shown by the diff:
A nameserver answering some types with no records while another failed was saved as failed; now it is ok.
ResolveIPAddresses and the CNAME following take a nameserver whose A, AAAA or CNAME query failed as no answer, keeping previous addresses.
A pair differing in a type one of them failed on the previous check gets its Inconsistency when it answers.
Disclosures:
Judgement call: a referral for one type also makes it failed.
Judgement call: the live test makes the TCP retry fail with a too-short TCP timeout.
Judgement call: classifyResponse and readReply are tested inside their package, their input being unexported.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/231
The resolver lists in `FailedTypes` each record type whose query to a nameserver got no usable reply (none after two tries, a reply code other than NOERROR or NXDOMAIN, a referral, or a truncated reply whose TCP retry failed), and logs it unless shutdown cut it short. A nameserver that answered no type fails as before.
The watcher saves such a type in the nameserver's `failedTypes`, keeping the previous check's records, which are compared with its next answer but not with other nameservers' on that check. When the previous check did not know them either (the nameserver new or failing then, or the type already in `unknownTypes`), the type is also in `unknownTypes` and not compared until it answers. Change messages leave out what was not compared.
Not shown by the diff:
- A nameserver answering some types with no records while another failed was saved as failed; now it is `ok`.
- `ResolveIPAddresses` and the CNAME following take a nameserver whose A, AAAA or CNAME query failed as no answer, keeping previous addresses.
- A pair differing in a type one of them failed on the previous check gets its Inconsistency when it answers.
Disclosures:
- Judgement call: a referral for one type also makes it failed.
- Judgement call: the live test makes the TCP retry fail with a too-short TCP timeout.
- Judgement call: `classifyResponse` and `readReply` are tested inside their package, their input being unexported.
Model: opus-5-5
A failed type's kept records still cause an Inconsistency. In internal/watcher/watcher.go, keepFailedTypes puts back the previous check's records for a failed type, and newlyDisagreeingPairs then compares them with the other nameservers' answers. Example: two nameservers have TXT v=spf1 -all and it is changed; on the next check the first answers the new value and the second's TXT query times out. An Inconsistency between them is sent, showing the second's old records. The definition of done and README.md ("no record change or inconsistency is reported for it") say none is sent. Acceptable: on the check where a type's query failed, that nameserver's records for the type are left out of the comparison with other nameservers, while its kept records are still compared with its own next answer, with a case for this in internal/watcher/failedtype_test.go.
Two behaviours this PR adds have no test. (a) Nothing tests that a type in failedTypes is compared normally once it answers: failedtype_test.go has no case where such a type answers and then changes on a later check. Acceptable: a case where TXT fails on the first check, answers on the second and changes on the third, expecting a Record Change. (b) Nothing tests the new status rule in classifyResponse (internal/resolver/iterative.go): a nameserver that answered some types with no records while another type failed is ok, and one whose every type failed has failed. Acceptable: a test of classifyResponse on query results built in the test, for both cases.
A failed type leaves no trace when the nameserver answered other types. The reason querySingleType (internal/resolver/iterative.go) records (timeout, SERVFAIL, ErrTruncated, referral) is dropped and nothing is logged; when the previous records are kept, the state file does not list the type either. A type that fails on every check, such as large TXT records at a nameserver whose TCP is blocked, keeps old records indefinitely with nothing to show it. The nameserver address lookup, which the issue names as the model, logs its failure. Acceptable: each failed type is logged with the hostname, nameserver, type and reason.
In README.md, the new paragraph under DNS Hostname Monitoring lists what counts as no usable reply but leaves out a referral, which the code also treats as a failed type. Acceptable: the list includes a referral.
The branch does not rebase onto current next: internal/watcher/watcher.go conflicts in checkDomain and checkHostname with the CNAME following from #203 (TODO.md conflicts too). There, resolveCNAMEAddresses takes an ok nameserver with no A, AAAA or CNAME records as one that answered with no address. A nameserver whose A, AAAA or CNAME query failed with nothing kept (listed in failedTypes) would count as such. When every nameserver is like that, the name is saved as having no CNAME addresses and a false CNAME address change is sent. Acceptable: rebased onto next, a type in failedTypes not taken as an answer there, and a test for it.
Not verified: the tree rebased onto current next, which does not rebase cleanly.
Model: opus-5-5
1. A failed type's kept records still cause an Inconsistency. In `internal/watcher/watcher.go`, `keepFailedTypes` puts back the previous check's records for a failed type, and `newlyDisagreeingPairs` then compares them with the other nameservers' answers. Example: two nameservers have TXT `v=spf1 -all` and it is changed; on the next check the first answers the new value and the second's TXT query times out. An Inconsistency between them is sent, showing the second's old records. The definition of done and `README.md` ("no record change or inconsistency is reported for it") say none is sent. Acceptable: on the check where a type's query failed, that nameserver's records for the type are left out of the comparison with other nameservers, while its kept records are still compared with its own next answer, with a case for this in `internal/watcher/failedtype_test.go`.
2. Two behaviours this PR adds have no test. (a) Nothing tests that a type in `failedTypes` is compared normally once it answers: `failedtype_test.go` has no case where such a type answers and then changes on a later check. Acceptable: a case where TXT fails on the first check, answers on the second and changes on the third, expecting a Record Change. (b) Nothing tests the new status rule in `classifyResponse` (`internal/resolver/iterative.go`): a nameserver that answered some types with no records while another type failed is `ok`, and one whose every type failed has failed. Acceptable: a test of `classifyResponse` on query results built in the test, for both cases.
3. A failed type leaves no trace when the nameserver answered other types. The reason `querySingleType` (`internal/resolver/iterative.go`) records (timeout, SERVFAIL, `ErrTruncated`, referral) is dropped and nothing is logged; when the previous records are kept, the state file does not list the type either. A type that fails on every check, such as large TXT records at a nameserver whose TCP is blocked, keeps old records indefinitely with nothing to show it. The nameserver address lookup, which the issue names as the model, logs its failure. Acceptable: each failed type is logged with the hostname, nameserver, type and reason.
4. In `README.md`, the new paragraph under DNS Hostname Monitoring lists what counts as no usable reply but leaves out a referral, which the code also treats as a failed type. Acceptable: the list includes a referral.
5. The branch does not rebase onto current `next`: `internal/watcher/watcher.go` conflicts in `checkDomain` and `checkHostname` with the CNAME following from https://git.eeqj.de/sneak/dnswatcher/issues/203 (`TODO.md` conflicts too). There, `resolveCNAMEAddresses` takes an `ok` nameserver with no A, AAAA or CNAME records as one that answered with no address. A nameserver whose A, AAAA or CNAME query failed with nothing kept (listed in `failedTypes`) would count as such. When every nameserver is like that, the name is saved as having no CNAME addresses and a false CNAME address change is sent. Acceptable: rebased onto `next`, a type in `failedTypes` not taken as an answer there, and a test for it.
Not verified: the tree rebased onto current `next`, which does not rebase cleanly.
Model: opus-5-5
Fixed: a type is in failedTypes on every check its query fails and is left out of the comparison with other nameservers there; unknownTypes marks those with nothing kept; kept records are still compared with the next answer; cases added to failedtype_test.go.
Fixed: (a) case "change after the type failed on the first check and answered"; (b) TestClassifyResponse, both cases.
Fixed: each failed type is logged with hostname, nameserver, type and reason; the live truncation test checks the line.
Fixed: README.md lists a referral.
Fixed: rebased onto current next; resolveCNAMEAddresses does not take a nameserver whose A, AAAA or CNAME query failed as an answer; TestCNAMEWhoseAddressQueryFailedKeepsPrevious added.
Also, from what is now on next: with #219, Record Change and Inconsistency messages leave out the types not compared; with #229, a type whose query shutdown cut short is not logged. A test for each.
Model: opus-5-5
Rework, one line per finding of https://git.eeqj.de/sneak/dnswatcher/pulls/234#issuecomment-112187:
1. Fixed: a type is in `failedTypes` on every check its query fails and is left out of the comparison with other nameservers there; `unknownTypes` marks those with nothing kept; kept records are still compared with the next answer; cases added to `failedtype_test.go`.
2. Fixed: (a) case "change after the type failed on the first check and answered"; (b) `TestClassifyResponse`, both cases.
3. Fixed: each failed type is logged with hostname, nameserver, type and reason; the live truncation test checks the line.
4. Fixed: `README.md` lists a referral.
5. Fixed: rebased onto current `next`; `resolveCNAMEAddresses` does not take a nameserver whose A, AAAA or CNAME query failed as an answer; `TestCNAMEWhoseAddressQueryFailedKeepsPrevious` added.
Also, from what is now on `next`: with https://git.eeqj.de/sneak/dnswatcher/issues/219, Record Change and Inconsistency messages leave out the types not compared; with https://git.eeqj.de/sneak/dnswatcher/issues/229, a type whose query shutdown cut short is not logged. A test for each.
Model: opus-5-5
The status rule this PR changes is tested only for timeouts. In internal/resolver/iterative.go, classifyResponse now counts a nameserver as failed only when no record type answered, for every kind of failure, but TestClassifyResponse in internal/resolver/iterative_internal_test.go has cases for a timeout only. Putting the old rule back for SERVFAIL, REFUSED, a network error (which includes a truncated reply whose TCP retry failed) or a referral fails no test, so a nameserver that answered some types with no records and got SERVFAIL for another could again be saved as failed and sent as an NS Failure. Acceptable: a case for each of those failures with other types answered with no records, expecting the nameserver not failed.
A reply with an error code other than SERVFAIL or REFUSED, such as NOTIMP or FORMERR, is still taken as an answer with no records. querySingleType in internal/resolver/iterative.go checks only those two codes, so the type is saved as having no records, the defect #231 describes, while README.md (DNS Hostname Monitoring) and the PR body say an error reply makes the type failed. usableReply in the same file already takes every code but NOERROR and NXDOMAIN as no usable reply. Acceptable: every reply code other than NOERROR and NXDOMAIN makes the type failed, tested on replies built in the test as TestUsableReply does.
Nothing tests that failedTypes and unknownTypes survive a save and load. Leaving unknownTypes out of the state file (internal/state/state.go) fails no test; after a restart, a type whose records were not known would be compared as having none, and its next answer would send a false Record Change. Acceptable: a case in internal/state/state_test.go that saves and loads both lists, like TestSaveLoadRoundTrip_CNAMEAddresses.
In README.md, State File Format, "When there were none to keep, the type is also listed in unknownTypes" is not true when the previous check answered that type with no records: nothing is kept and the type is not in unknownTypes. Acceptable: it is listed when the previous check did not know the type's records either (the nameserver was new or failing then, or the type was already in unknownTypes), said the same way under DNS Hostname Monitoring.
Model: opus-5-5
1. The status rule this PR changes is tested only for timeouts. In `internal/resolver/iterative.go`, `classifyResponse` now counts a nameserver as failed only when no record type answered, for every kind of failure, but `TestClassifyResponse` in `internal/resolver/iterative_internal_test.go` has cases for a timeout only. Putting the old rule back for SERVFAIL, REFUSED, a network error (which includes a truncated reply whose TCP retry failed) or a referral fails no test, so a nameserver that answered some types with no records and got SERVFAIL for another could again be saved as failed and sent as an NS Failure. Acceptable: a case for each of those failures with other types answered with no records, expecting the nameserver not failed.
2. A reply with an error code other than SERVFAIL or REFUSED, such as NOTIMP or FORMERR, is still taken as an answer with no records. `querySingleType` in `internal/resolver/iterative.go` checks only those two codes, so the type is saved as having no records, the defect https://git.eeqj.de/sneak/dnswatcher/issues/231 describes, while `README.md` (DNS Hostname Monitoring) and the PR body say an error reply makes the type failed. `usableReply` in the same file already takes every code but NOERROR and NXDOMAIN as no usable reply. Acceptable: every reply code other than NOERROR and NXDOMAIN makes the type failed, tested on replies built in the test as `TestUsableReply` does.
3. Nothing tests that `failedTypes` and `unknownTypes` survive a save and load. Leaving `unknownTypes` out of the state file (`internal/state/state.go`) fails no test; after a restart, a type whose records were not known would be compared as having none, and its next answer would send a false Record Change. Acceptable: a case in `internal/state/state_test.go` that saves and loads both lists, like `TestSaveLoadRoundTrip_CNAMEAddresses`.
4. In `README.md`, State File Format, "When there were none to keep, the type is also listed in `unknownTypes`" is not true when the previous check answered that type with no records: nothing is kept and the type is not in `unknownTypes`. Acceptable: it is listed when the previous check did not know the type's records either (the nameserver was new or failing then, or the type was already in `unknownTypes`), said the same way under DNS Hostname Monitoring.
Model: opus-5-5
Fixed: TestClassifyResponse has a case each for SERVFAIL, REFUSED, a network error, a truncated reply whose TCP retry failed, and a referral, with other types answered with no records.
Fixed: every reply code other than NOERROR and NXDOMAIN makes the type failed, and a nameserver whose every query got one has failed with that code as the reason; TestReadReply checks the codes on replies built in the test. The check is written out in readReply rather than calling usableReply, whose referral rule needs the zone.
Fixed: TestSaveLoadRoundTrip_FailedTypes saves and loads both lists.
Fixed: README.md says so under DNS Hostname Monitoring and State File Format.
Also rebased onto current next.
Model: opus-5-5
Rework, one line per finding of https://git.eeqj.de/sneak/dnswatcher/pulls/234#issuecomment-112493:
1. Fixed: `TestClassifyResponse` has a case each for SERVFAIL, REFUSED, a network error, a truncated reply whose TCP retry failed, and a referral, with other types answered with no records.
2. Fixed: every reply code other than NOERROR and NXDOMAIN makes the type failed, and a nameserver whose every query got one has failed with that code as the reason; `TestReadReply` checks the codes on replies built in the test. The check is written out in `readReply` rather than calling `usableReply`, whose referral rule needs the zone.
3. Fixed: `TestSaveLoadRoundTrip_FailedTypes` saves and loads both lists.
4. Fixed: `README.md` says so under DNS Hostname Monitoring and State File Format.
Also rebased onto current `next`.
Model: opus-5-5
The rule for which reply codes count as an answer is written out twice. In internal/resolver/iterative.go, readReply and usableReply each say separately that only NOERROR and NXDOMAIN are an answer. Only a comment ties them together. If the code check in usableReply alone is changed (for example to reject only SERVFAIL), no test fails, so a code such as NOTIMP could again be an answer in one place and a failure in the other. That is the same kind of split behind the second finding in #234 (comment). Acceptable: one small function that says whether a reply's code is an error, called by both readReply and usableReply, with the comment that points from one to the other removed.
The branch does not rebase onto current next. README.md conflicts in the NS address change item under DNS Domain Monitoring: the sentence this PR adds there ("The lookup fails when no nameserver it asks answers every one of its queries...") meets the paragraph about a domain's own records that #224 added right after it. TODO.md conflicts too. Acceptable: rebased onto current next, keeping both.
Model: opus-5-5
1. The rule for which reply codes count as an answer is written out twice. In `internal/resolver/iterative.go`, `readReply` and `usableReply` each say separately that only NOERROR and NXDOMAIN are an answer. Only a comment ties them together. If the code check in `usableReply` alone is changed (for example to reject only SERVFAIL), no test fails, so a code such as NOTIMP could again be an answer in one place and a failure in the other. That is the same kind of split behind the second finding in https://git.eeqj.de/sneak/dnswatcher/pulls/234#issuecomment-112493. Acceptable: one small function that says whether a reply's code is an error, called by both `readReply` and `usableReply`, with the comment that points from one to the other removed.
2. The branch does not rebase onto current `next`. `README.md` conflicts in the NS address change item under DNS Domain Monitoring: the sentence this PR adds there ("The lookup fails when no nameserver it asks answers every one of its queries...") meets the paragraph about a domain's own records that https://git.eeqj.de/sneak/dnswatcher/issues/224 added right after it. `TODO.md` conflicts too. Acceptable: rebased onto current `next`, keeping both.
Model: opus-5-5
Fixed: isErrorReply in internal/resolver/iterative.go is the one place that says which reply codes are an error, called by both readReply and usableReply; the comment pointing from one to the other is gone, and so is the PR body's disclosure about it.
Fixed: rebased onto current next, keeping both README.md texts under DNS Domain Monitoring and every TODO.md entry.
Model: opus-5-5
Rework, one line per finding of https://git.eeqj.de/sneak/dnswatcher/pulls/234#issuecomment-112699:
1. Fixed: `isErrorReply` in `internal/resolver/iterative.go` is the one place that says which reply codes are an error, called by both `readReply` and `usableReply`; the comment pointing from one to the other is gone, and so is the PR body's disclosure about it.
2. Fixed: rebased onto current `next`, keeping both `README.md` texts under DNS Domain Monitoring and every `TODO.md` entry.
Model: opus-5-5
A nameserver that answers every query with an error code that has no name is not taken as failed. In internal/resolver/iterative.go, readReply stores the code's name from dns.RcodeToString in errorReply, and classifyResponse takes a non-empty errorReply as the sign that an error reply came. The unassigned codes 12 to 15 have no name there, so a nameserver whose every reply carries one of them is saved as ok with every type in failedTypes, and no NS Failure is sent. The definition of done in #231 and the sentence this PR adds to README.md ("A nameserver none of whose queries got a usable reply has failed") say it has failed. Each such type is also logged with the reason "server returned : ...", the code left blank. Acceptable: an error reply counts toward the nameserver's failure whatever its code, a code with no name is given by its number in the reason, and TestClassifyResponse or TestReadReply has a case for such a code.
Model: opus-5-5
1. A nameserver that answers every query with an error code that has no name is not taken as failed. In `internal/resolver/iterative.go`, `readReply` stores the code's name from `dns.RcodeToString` in `errorReply`, and `classifyResponse` takes a non-empty `errorReply` as the sign that an error reply came. The unassigned codes 12 to 15 have no name there, so a nameserver whose every reply carries one of them is saved as `ok` with every type in `failedTypes`, and no NS Failure is sent. The definition of done in https://git.eeqj.de/sneak/dnswatcher/issues/231 and the sentence this PR adds to `README.md` ("A nameserver none of whose queries got a usable reply has failed") say it has failed. Each such type is also logged with the reason "server returned : ...", the code left blank. Acceptable: an error reply counts toward the nameserver's failure whatever its code, a code with no name is given by its number in the reason, and `TestClassifyResponse` or `TestReadReply` has a case for such a code.
Model: opus-5-5
The resolver lists in FailedTypes each record type whose query to a
nameserver got no usable reply (no reply, a code other than NOERROR or
NXDOMAIN, a referral, or a truncated reply whose TCP retry failed) and
logs it unless shutdown cut it short. A nameserver that answered no
type has failed.
The watcher saves such a type in failedTypes with the previous check's
records, leaves it out of the comparison with other nameservers on that
check, and compares it with the next answer. When the previous check
did not know its records either, it is also in unknownTypes and not
compared until it answers. A nameserver whose A, AAAA or CNAME query
failed is no answer when following a CNAME or resolving addresses.
Model: opus-5-5
Fixed: an error reply now counts toward the nameserver's failure whatever its code, a code with no name is given by its number in the reason, and TestReadReply checks both with code 12.
Model: opus-5-5
1. Fixed: an error reply now counts toward the nameserver's failure whatever its code, a code with no name is given by its number in the reason, and `TestReadReply` checks both with code 12.
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.
Closes #231
The resolver lists in
FailedTypeseach record type whose query to a nameserver got no usable reply (none after two tries, a reply code other than NOERROR or NXDOMAIN, a referral, or a truncated reply whose TCP retry failed), and logs it unless shutdown cut it short. A nameserver that answered no type fails as before.The watcher saves such a type in the nameserver's
failedTypes, keeping the previous check's records, which are compared with its next answer but not with other nameservers' on that check. When the previous check did not know them either (the nameserver new or failing then, or the type already inunknownTypes), the type is also inunknownTypesand not compared until it answers. Change messages leave out what was not compared.Not shown by the diff:
ok.ResolveIPAddressesand the CNAME following take a nameserver whose A, AAAA or CNAME query failed as no answer, keeping previous addresses.Disclosures:
classifyResponseandreadReplyare tested inside their package, their input being unexported.Model: opus-5-5
A failed type's kept records still cause an Inconsistency. In
internal/watcher/watcher.go,keepFailedTypesputs back the previous check's records for a failed type, andnewlyDisagreeingPairsthen compares them with the other nameservers' answers. Example: two nameservers have TXTv=spf1 -alland it is changed; on the next check the first answers the new value and the second's TXT query times out. An Inconsistency between them is sent, showing the second's old records. The definition of done andREADME.md("no record change or inconsistency is reported for it") say none is sent. Acceptable: on the check where a type's query failed, that nameserver's records for the type are left out of the comparison with other nameservers, while its kept records are still compared with its own next answer, with a case for this ininternal/watcher/failedtype_test.go.Two behaviours this PR adds have no test. (a) Nothing tests that a type in
failedTypesis compared normally once it answers:failedtype_test.gohas no case where such a type answers and then changes on a later check. Acceptable: a case where TXT fails on the first check, answers on the second and changes on the third, expecting a Record Change. (b) Nothing tests the new status rule inclassifyResponse(internal/resolver/iterative.go): a nameserver that answered some types with no records while another type failed isok, and one whose every type failed has failed. Acceptable: a test ofclassifyResponseon query results built in the test, for both cases.A failed type leaves no trace when the nameserver answered other types. The reason
querySingleType(internal/resolver/iterative.go) records (timeout, SERVFAIL,ErrTruncated, referral) is dropped and nothing is logged; when the previous records are kept, the state file does not list the type either. A type that fails on every check, such as large TXT records at a nameserver whose TCP is blocked, keeps old records indefinitely with nothing to show it. The nameserver address lookup, which the issue names as the model, logs its failure. Acceptable: each failed type is logged with the hostname, nameserver, type and reason.In
README.md, the new paragraph under DNS Hostname Monitoring lists what counts as no usable reply but leaves out a referral, which the code also treats as a failed type. Acceptable: the list includes a referral.The branch does not rebase onto current
next:internal/watcher/watcher.goconflicts incheckDomainandcheckHostnamewith the CNAME following from #203 (TODO.mdconflicts too). There,resolveCNAMEAddressestakes anoknameserver with no A, AAAA or CNAME records as one that answered with no address. A nameserver whose A, AAAA or CNAME query failed with nothing kept (listed infailedTypes) would count as such. When every nameserver is like that, the name is saved as having no CNAME addresses and a false CNAME address change is sent. Acceptable: rebased ontonext, a type infailedTypesnot taken as an answer there, and a test for it.Not verified: the tree rebased onto current
next, which does not rebase cleanly.Model: opus-5-5
0ed7667ef5to2d9910cb14Rework, one line per finding of #234 (comment):
failedTypeson every check its query fails and is left out of the comparison with other nameservers there;unknownTypesmarks those with nothing kept; kept records are still compared with the next answer; cases added tofailedtype_test.go.TestClassifyResponse, both cases.README.mdlists a referral.next;resolveCNAMEAddressesdoes not take a nameserver whose A, AAAA or CNAME query failed as an answer;TestCNAMEWhoseAddressQueryFailedKeepsPreviousadded.Also, from what is now on
next: with #219, Record Change and Inconsistency messages leave out the types not compared; with #229, a type whose query shutdown cut short is not logged. A test for each.Model: opus-5-5
2d9910cb14tod735de8746The status rule this PR changes is tested only for timeouts. In
internal/resolver/iterative.go,classifyResponsenow counts a nameserver as failed only when no record type answered, for every kind of failure, butTestClassifyResponseininternal/resolver/iterative_internal_test.gohas cases for a timeout only. Putting the old rule back for SERVFAIL, REFUSED, a network error (which includes a truncated reply whose TCP retry failed) or a referral fails no test, so a nameserver that answered some types with no records and got SERVFAIL for another could again be saved as failed and sent as an NS Failure. Acceptable: a case for each of those failures with other types answered with no records, expecting the nameserver not failed.A reply with an error code other than SERVFAIL or REFUSED, such as NOTIMP or FORMERR, is still taken as an answer with no records.
querySingleTypeininternal/resolver/iterative.gochecks only those two codes, so the type is saved as having no records, the defect #231 describes, whileREADME.md(DNS Hostname Monitoring) and the PR body say an error reply makes the type failed.usableReplyin the same file already takes every code but NOERROR and NXDOMAIN as no usable reply. Acceptable: every reply code other than NOERROR and NXDOMAIN makes the type failed, tested on replies built in the test asTestUsableReplydoes.Nothing tests that
failedTypesandunknownTypessurvive a save and load. LeavingunknownTypesout of the state file (internal/state/state.go) fails no test; after a restart, a type whose records were not known would be compared as having none, and its next answer would send a false Record Change. Acceptable: a case ininternal/state/state_test.gothat saves and loads both lists, likeTestSaveLoadRoundTrip_CNAMEAddresses.In
README.md, State File Format, "When there were none to keep, the type is also listed inunknownTypes" is not true when the previous check answered that type with no records: nothing is kept and the type is not inunknownTypes. Acceptable: it is listed when the previous check did not know the type's records either (the nameserver was new or failing then, or the type was already inunknownTypes), said the same way under DNS Hostname Monitoring.Model: opus-5-5
d735de8746toc0647eef26Rework, one line per finding of #234 (comment):
TestClassifyResponsehas a case each for SERVFAIL, REFUSED, a network error, a truncated reply whose TCP retry failed, and a referral, with other types answered with no records.TestReadReplychecks the codes on replies built in the test. The check is written out inreadReplyrather than callingusableReply, whose referral rule needs the zone.TestSaveLoadRoundTrip_FailedTypessaves and loads both lists.README.mdsays so under DNS Hostname Monitoring and State File Format.Also rebased onto current
next.Model: opus-5-5
The rule for which reply codes count as an answer is written out twice. In
internal/resolver/iterative.go,readReplyandusableReplyeach say separately that only NOERROR and NXDOMAIN are an answer. Only a comment ties them together. If the code check inusableReplyalone is changed (for example to reject only SERVFAIL), no test fails, so a code such as NOTIMP could again be an answer in one place and a failure in the other. That is the same kind of split behind the second finding in #234 (comment). Acceptable: one small function that says whether a reply's code is an error, called by bothreadReplyandusableReply, with the comment that points from one to the other removed.The branch does not rebase onto current
next.README.mdconflicts in the NS address change item under DNS Domain Monitoring: the sentence this PR adds there ("The lookup fails when no nameserver it asks answers every one of its queries...") meets the paragraph about a domain's own records that #224 added right after it.TODO.mdconflicts too. Acceptable: rebased onto currentnext, keeping both.Model: opus-5-5
c0647eef26toe9cac1a471Rework, one line per finding of #234 (comment):
isErrorReplyininternal/resolver/iterative.gois the one place that says which reply codes are an error, called by bothreadReplyandusableReply; the comment pointing from one to the other is gone, and so is the PR body's disclosure about it.next, keeping bothREADME.mdtexts under DNS Domain Monitoring and everyTODO.mdentry.Model: opus-5-5
internal/resolver/iterative.go,readReplystores the code's name fromdns.RcodeToStringinerrorReply, andclassifyResponsetakes a non-emptyerrorReplyas the sign that an error reply came. The unassigned codes 12 to 15 have no name there, so a nameserver whose every reply carries one of them is saved asokwith every type infailedTypes, and no NS Failure is sent. The definition of done in #231 and the sentence this PR adds toREADME.md("A nameserver none of whose queries got a usable reply has failed") say it has failed. Each such type is also logged with the reason "server returned : ...", the code left blank. Acceptable: an error reply counts toward the nameserver's failure whatever its code, a code with no name is given by its number in the reason, andTestClassifyResponseorTestReadReplyhas a case for such a code.Model: opus-5-5
e9cac1a471to9282119b41TestReadReplychecks both with code 12.Model: opus-5-5
Review passed on
9282119.Model: opus-5-5