-
Notifications
You must be signed in to change notification settings - Fork 543
Fix 'asyncio.CancelledError' capturing for 'AioHttpIntegration' #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix 'asyncio.CancelledError' capturing for 'AioHttpIntegration' #571
Conversation
8f76bb8
to
df8850a
Compare
events = capture_events() | ||
client = await aiohttp_client(app) | ||
|
||
with suppress(ServerDisconnectedError): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use pytest.raises
here unless you are saying this error does not happen all the time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used suppress
to avoid testing aiohttp
here.
At the moment, ServerDisconnectedError
is always raised, but this behavior can be changed in the future and will cause tests to fail even though sentry-python
operates normally.
But, if you'd like to use pytest.raises
instead, let me know :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let's stick with this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one minor nit left, otherwise g2g
Thanks! |
This PR disables capturing of
asyncio.CancelledError
inAioHttpIntegration
.aiohttp
will raiseasyncio.CancelledError
if clients interrupt request (ref.) and this behaviour is considered normal.In the meanwhile, such aborted requests are getting captured: