https_client: log recoverable connection errors at INFO instead of WARNING - #211
Closed
MrObvious wants to merge 3 commits into
Closed
https_client: log recoverable connection errors at INFO instead of WARNING#211MrObvious wants to merge 3 commits into
MrObvious wants to merge 3 commits into
Conversation
…errors, not just timeouts We saw recurring "curl request failed with 16: Error" (CURLE_HTTP2) and "curl request failed with 55: Error / Send failure: Broken pipe" (CURLE_SEND_ERROR) on a production OpenWrt router running two https-dns-proxy instances, even after tuning max_idle_time down to 30s. These are classic symptoms of curl reusing a stale/half-closed HTTP/2 connection - the same class of problem the existing reset_timer/https_client_reset mechanism was built to recover from, but that mechanism is currently only armed on CURLE_OPERATION_TIMEDOUT. HTTP/2 stream errors and send/recv errors on a reused connection never reach it, so the proxy keeps trying to reuse a bad connection indefinitely instead of forcing a fresh one. This extends the existing case in https_fetch_ctx_process_response() to also arm the reset timer for CURLE_HTTP2, CURLE_HTTP2_STREAM, CURLE_GOT_NOTHING, and CURLE_SEND_ERROR - all indicators of a broken/stale connection rather than a one-off content error. No new mechanism is introduced; this only widens the set of error codes that trigger the recovery path that already exists. Tested on a production OpenWrt router (two instances, Cloudflare + Quad9 backends) - confirmed the reset timer now arms and fires (full client reset) in response to these errors, where previously they were silently ignored.
…RNING Follow-up to aarond10#210 (reset-timer fix). Submitted as a separate change since this is a logging-policy question rather than a behavior fix, and should not hold up the reset-timer fix if there is disagreement on log levels. CURLE_OPERATION_TIMEDOUT, CURLE_HTTP2, CURLE_HTTP2_STREAM, CURLE_GOT_NOTHING, and CURLE_SEND_ERROR are all connection-reuse hiccups that the reset-timer path (see aarond10#210) already recovers from automatically without any user-visible impact. On resolvers whose edge cycles persistent connections periodically (observed with Quad9, not seen with Cloudflare), this is a routine, expected, self-healing condition - not a warning-worthy event - yet it was logged at WARNING twice per occurrence. On flash- and memory-constrained routers running at default verbosity, this produces a steady stream of log noise for a condition the proxy is already handling correctly on its own. This logs the same information at INFO instead of WARNING specifically for the recoverable error codes above, matching the existing INFO-level "Client reset timer started" message already used for this recovery path. Genuinely unexpected failures are unchanged and still log at WARNING. Tested on the same production router as aarond10#210 - confirmed curl-16 spam disappears from default-verbosity logs while the reset-timer recovery (and its own INFO-level messages, visible at raised verbosity) continues to function.
Owner
|
Sorry. I pushed both these manually as I managed to bump something and added an unwanted merge to the PR. |
Contributor
Author
|
Ok thanks Aaron. So both changes did get merged? That’s all I care about. Appreciate the quick turnaround. |
Owner
|
Yes. Both landed.
…On Fri, 31 Jul 2026 at 00:18, MrObvious ***@***.***> wrote:
*MrObvious* left a comment (aarond10/https_dns_proxy#211)
<#211 (comment)>
Ok thanks Aaron. So both changes did get merged? That’s all I care about.
Appreciate the quick turnaround.
—
Reply to this email directly, view it on GitHub
<#211?email_source=notifications&email_token=AABTOXQHOSTYJGGIPZP4KH35HNKJVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJTGE4TQNJUGEZ2M4TFMFZW63VMON2GC5DFL5RWQYLOM5S2KZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5131985413>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABTOXX7MJDBAAR53I23Q635HNKJVAVCNFSNUABEKJSXA33TNF2G64TZHM2TOMJSHA2TGOJ3JFZXG5LFHM2TAMJTG4YTENRVGKQXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AABTOXQBVVDZPNRJT75JVML5HNKJVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJTGE4TQNJUGEZ2M4TFMFZW63VMON2GC5DFL5RWQYLOM5S2KZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/AABTOXREUVOW53GKHGGV7FD5HNKJVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJTGE4TQNJUGEZ2M4TFMFZW63VMON2GC5DFL5RWQYLOM5S2KZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Contributor
Author
|
Thank you! To quote the kids these days, that’s bussin! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #210 (reset-timer fix), submitted as a separate PR since this is a logging-policy question rather than a behavior fix and should not block the reset-timer fix if there is disagreement here. This branch is stacked on #210 (2 commits shown), so this diff will reduce to just the second commit once #210 merges.
CURLE_OPERATION_TIMEDOUT, CURLE_HTTP2, CURLE_HTTP2_STREAM, CURLE_GOT_NOTHING, and CURLE_SEND_ERROR are all connection-reuse hiccups that the reset-timer path (#210) already recovers from automatically without any user-visible impact. On resolvers whose edge cycles persistent connections periodically (observed with Quad9, not seen with Cloudflare), this is a routine, expected, self-healing condition - not a warning-worthy event - yet it was logged at WARNING twice per occurrence. On flash- and memory-constrained routers running at default verbosity, this produces a steady stream of log noise for a condition the proxy is already handling correctly on its own.
This logs the same information at INFO instead of WARNING specifically for the recoverable error codes above, matching the existing INFO-level "Client reset timer started" message already used for this recovery path. Genuinely unexpected failures are unchanged and still log at WARNING.
Tested on the same production router as #210 - confirmed curl-16 spam disappears from default-verbosity logs while the reset-timer recovery (and its own INFO-level messages, visible at raised verbosity) continues to function.