async client: retry on bare ServerDisconnectedError#726
Merged
joe-clickhouse merged 1 commit intoClickHouse:mainfrom Apr 30, 2026
Merged
Conversation
aiohttp raises ServerDisconnectedError without a message when a pooled keep-alive connection has been closed by the server (or an upstream load balancer) and the client picks it up on the next request. The existing retry path in _raw_request matched on "Connection reset" / "Remote end closed" / "Cannot connect" but not on the bare "Server disconnected" message, so the failure surfaced as OperationalError on the first post-idle request instead of being transparently retried. Add "Server disconnected" to the retry substring list and add async integration coverage in test_error_handling.py for both the retried-once and retry-also-fails paths, plus a regression check for the existing "Connection reset" branch.
|
|
joe-clickhouse
approved these changes
Apr 30, 2026
Contributor
joe-clickhouse
left a comment
joe-clickhouse
left a comment
There was a problem hiding this comment.
Looks good. Thanks!
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
3 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.
Summary
The async client's
_raw_requestretry path catchesaiohttp.ServerConnectionErrorand retries once whenstr(e)contains"Connection reset","Remote end closed", or"Cannot connect". It does not match the bareaiohttp.ServerDisconnectedError()whose default message is"Server disconnected", which is what aiohttp raises in current versions when a pooled keep-alive connection has been closed by the server (or an upstream load balancer) between requests. The first request after an idle period reuses the stale connection, sees the disconnect, and surfaces asOperationalError("Network Error: Server disconnected")to the caller instead of being transparently retried on a fresh connection.The fix adds
"Server disconnected"to the existing substring list. Reproduces and verifies intests/integration_tests/test_error_handling.pyagainst a real ClickHouse server using thetest_native_async_clientfixture andpytest-mockto inject the disconnect at the_session.requestboundary.Tests added:
test_async_retry_on_server_disconnected: bareServerDisconnectedError()is retried once and the second attempt succeeds. Fails onmain, passes with this change.test_async_server_disconnected_raises_after_retry: if the disconnect persists,OperationalError("Network Error: Server disconnected")still surfaces with the originalServerDisconnectedErroras__cause__.test_async_retry_on_connection_reset: regression check that the pre-existing"Connection reset"retry branch still works.This is async-only. The sync client uses urllib3, which retries pooled-connection drops independently via
Retry.Closes #725
Checklist
Delete items not relevant to your PR: