Repository navigation
fix: close the CCIP-read aiohttp session on every exit path - #3913
Open
franco-gondi wants to merge 1 commit into
Open
franco-gondi wants to merge 1 commit into
franco-gondi wants to merge 1 commit into
Conversation
async_handle_offchain_lookup opened a bare ClientSession() and closed it by hand before each return and raise. Any exception it did not anticipate skipped the close and leaked the session together with its pooled connection, which asyncio then logs as "Unclosed client session" and "Unclosed connector". For example: response.json() raising ContentTypeError on a 2xx gateway reply that is not JSON, a JSON body that is not a dict, or cancellation. Wrapping the session in `async with` closes it on every path. Adds a regression test where the gateway response's json() raises. Claude-Session: https://claude.ai/code/session_01LDYugMbgucFoMVbtMY5SJx
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
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.
What was wrong?
async_handle_offchain_lookup(the async CCIP-read / EIP-3668 handler) opened a bareClientSession()and closed it by hand before eachreturnandraise. An exception it did not anticipate skipped the close and leaked the session along with its pooled connection. asyncio then logsUnclosed client sessionandUnclosed connectorwhen the session is garbage-collected.The common trigger is a gateway that answers 2xx with a body that isn't JSON (e.g.
text/html).await response.json()raisesaiohttp.ContentTypeErrorafter the body has been read and the connection released to the pool. A JSON body that isn't a dict, or a cancelled task, leaks the session the same way.How was it fixed?
The session now lives in
async with ClientSession() as session:, so it closes on every exit path, and the manualawait session.close()calls are gone. The rest of the function is only re-indented.Adds
test_async_offchain_resolution_closes_session_when_response_is_not_json. It mocks a gateway response whosejson()raises and asserts the session was closed. The test fails before the change and passes after.