Skip to content

Count an oversized DoH body as a failed attempt (#760) - #775

Merged
kasnder merged 1 commit into
masterfrom
fix/doh-oversized-attempt-accounting
Aug 22, 2026
Merged

Count an oversized DoH body as a failed attempt (#760)#775
kasnder merged 1 commit into
masterfrom
fix/doh-oversized-attempt-accounting

Conversation

@kasnder

@kasnder kasnder commented Aug 22, 2026

Copy link
Copy Markdown
Member

Closes #760.

DnsOverHttpsClient.resolve() reports every unusable response through
reportFailedAttempt() before retrying — a 5xx, a null body, a sub-12-byte
answer, a non-cancelled IOException. The two oversized-body branches added by
#771 were the exception: they continue without reporting.

DnsProxyServer charges a failed query Math.max(1, queryFailedAttempts.get()),
so this was not "never trips the breaker" — the floor of 1 still applies. But it
did mean an endpoint returning nothing but oversized bodies cost one failure
per query however many attempts it burned, so it took ten whole queries to reach
CIRCUIT_BREAKER_THRESHOLD where every other failure mode takes three. That is
exactly the per-attempt accounting #768 introduced, applied to the one path that
missed it.

Two lines, plus regression coverage for both the declared-Content-Length and
the chunked path (the latter is only caught after peekBody, so it needs its own
test).

What is not changed

The other half of #760dohFailures.set(0) on any success — is left alone
deliberately. #768 made each wasted attempt count, so a flapping endpoint now
trips after ~3–4 bad queries rather than needing ten consecutive clean failures,
which defuses the original report. Resetting on success is the fail-open
behaviour we want for a DNS path that must not stay broken.

Verification

  • ./gradlew :app:testGithubDebugUnitTest --tests '…DnsOverHttpsClientTest' — 24 tests, 0 failures.
  • Negative control: with the source change reverted and the tests kept, exactly
    the two new tests fail (expected:<3> but was:<0> and expected:<1> but was:<0>)
    and no others — so they test the fix rather than passing vacuously.

🤖 Generated with Claude Code

Every other unusable response in resolve() reports through
reportFailedAttempt() before retrying; the two oversized-body branches
did not. DnsProxyServer floors the per-query cost at
Math.max(1, queryFailedAttempts.get()), so an endpoint returning nothing
but oversized bodies was charged one failure per query however many
attempts it burned -- ten whole queries to trip the circuit breaker
where every other failure mode takes three.

Covers both the declared-Content-Length and the chunked path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kasnder
kasnder merged commit 66c71d8 into master Aug 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DoH circuit breaker counts endpoint-switch cancellations as failures and resets on any success

1 participant