Repository navigation
fix Future exception never retrieved in create_connection - #590
jensbjorgensen wants to merge 1 commit into
Conversation
fantix
left a comment
fantix
left a comment
There was a problem hiding this comment.
This makes sense! Though a unit test is preferred.
| except asyncio.CancelledError as exc: | ||
| for fut in fs: | ||
| fut.cancel() | ||
| raise exc from None |
There was a problem hiding this comment.
Why dropping the original context here, instead of just raise?
There was a problem hiding this comment.
Looks like I failed to loop back to this and take care of your requests. Just pushed a commit (replaced old one since it was now behind), I now just raise and I've also added a unit test.
|
Still happens on 0.22.1 and 0.23.0. Repro, the lookup fails after the caller has given up: import asyncio, gc, logging, sys
import uvloop
logging.basicConfig(level=logging.ERROR)
async def main():
loop = asyncio.get_running_loop()
try:
await asyncio.wait_for(loop.create_connection(asyncio.Protocol, "no-such-host.invalid", 443), timeout=0.001)
except (asyncio.TimeoutError, OSError) as e:
print("caller sees:", type(e).__name__)
await asyncio.sleep(2) # let the lookup fail
gc.collect()
await asyncio.sleep(0.1)
if sys.argv[1:] == ["stdlib"]:
asyncio.run(main())
else:
uvloop.run(main())0.23.0: With the stdlib loop ( I applied this PR's diff on top of v0.23.0 (it also still applies to current master) and built it: the warning is gone, also under |
When create_connection() is cancelled (e.g. by a wait_for() timeout)
while its getaddrinfo() lookups are still in flight, asyncio.wait()
leaves the lookup futures running. Once such a lookup fails, nobody
retrieves its exception and the loop logs:
Future exception was never retrieved
future: <Future finished exception=gaierror(-2, 'Name or service not known')>
Cancel the pending lookups when the wait is cancelled. A lookup that
has already finished can't be cancelled, so mark its exception (if any)
as retrieved instead.
The stdlib event loop doesn't have this problem because its lookup
future is awaited directly and therefore cancelled with the task.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
d464c58 to
f8e3a70
Compare
jensbjorgensen
left a comment
jensbjorgensen
left a comment
There was a problem hiding this comment.
switched to raise, unit test added
3 participants
I also have observed that when I use asyncio.wait_for(...) around loop.create_connection in the timeout case I end up getting this logged to stdout(err?):
The code in this commit fixes this.