ensure cli event loop lives long enough for all tasks to finish #1199
+53
−6
Conversation
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
…ather with return_exceptions to allow in-flight backoffs to finish - Retry ServerDisconnectedError/ServerTimeoutError/ClientConnectionError/asyncio.TimeoutError for GET/HEAD/OPTIONS/PROPFIND/REPORT - Keep original rate-limit handling (429, Google 403 usageLimits) - In CLI, avoid cancelling sibling tasks so per-request backoff can complete; re-raise first failure after all tasks finish
WhyNotHugo
force-pushed
the
rate-limiting-more
branch
from
7fbcfed to
977b9c0
Compare
|
Rebased with minor tweaks (mostly formatting). |
WhyNotHugo
reviewed
Oct 4, 2025
|
Seems to mess up some TLS tests: |
- `ClientConnectionError` in `aiohttp` can wrap SSL handshake and certificate verification errors - Retrying those hides the real cause and produced `TransientNetworkError` instead of the expected certificate error - Removing `ClientConnectionError` from the transient list lets SSL errors surface correctly
|
@WhyNotHugo tests now pass :) |
WhyNotHugo
approved these changes
Oct 9, 2025
WhyNotHugo
reviewed
Oct 9, 2025
Comment on lines
149
to
+157
|
|
||
| await asyncio.gather(*tasks) | ||
| # `return_exceptions=True` ensures that the event loop lives long enough for | ||
| # backoffs to be able to finish | ||
| gathered = await asyncio.gather(*tasks, return_exceptions=True) | ||
| # but now we need to manually check for and propogate a single failure after | ||
| # allowing all tasks to finish in order to keep exit status non-zero | ||
| failures = [e for e in gathered if isinstance(e, BaseException)] | ||
| if failures: | ||
| raise failures[0] |
There was a problem hiding this comment.
I just noticed that this is outside the async with aiohttp.TCPConnector(limit_per_host=16) as conn: block.
The connector will be closed by the time this block is reached, so it will likely still lead to #1126.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I believe this solves #1126
aiohttpdisconnectsServerDisconnectedError/ServerTimeoutError/ClientConnectionError/asyncio.TimeoutErrorforGET/HEAD/OPTIONS/PROPFIND/REPORT429, Google403 usageLimits)