Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Add the Client.install_asyncio_hook function. #1134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kentzo
Copy link

@Kentzo Kentzo commented Nov 12, 2017

asyncio needs special handling as its unhandled exceptions are consumed by logging and ignoring them.

@asvetlov Please take a look at the implementation and let me know if handling can be improved. I used implementation of BaseEventLoop.default_exception_handler as a reference but didn't check all available context's keywords in all released versions of Python.

@Kentzo Kentzo force-pushed the asyncio branch 5 times, most recently from 613df38 to 7439554 Compare November 12, 2017 06:09
@ashwoods ashwoods self-requested a review November 12, 2017 15:28
@ashwoods ashwoods self-assigned this Nov 12, 2017
@Kentzo
Copy link
Author

Kentzo commented Nov 13, 2017

I'm not sure how to cause asyncio to call exception handler without 'exception' but with handle of future with the _source_traceback attribute set.

@Kentzo
Copy link
Author

Kentzo commented Nov 13, 2017

@ashwoods I used the 'exception' level instead of 'fatal' as handled errors don't explicitly cause interpreter to exit by default. Was it a good idea?

@Kentzo
Copy link
Author

Kentzo commented Nov 20, 2017

@ashwoods Is there anything I should change / fix?

@asvetlov
Copy link
Contributor

The idea is strange for me.
Why is it a part of raven-python?
I thought raven-aiohttp is better place for asyncio-related things.

@Kentzo
Copy link
Author

Kentzo commented Nov 22, 2017

@asvetlov This change is to handle asyncio exceptions and unrelated to aiohttp transport.

In one of the projects we are using this without raven-aiohttp.

@Kentzo
Copy link
Author

Kentzo commented Jan 25, 2018

@ashwoods I would love to address the issues if there are any

@Kentzo
Copy link
Author

Kentzo commented Mar 30, 2018

Anyone? I'm willing to fix any issues.

@ashwoods
Copy link
Contributor

I haven't had time to look into context in the loop. Will breadcrumbs leak and be cleared?

@Kentzo
Copy link
Author

Kentzo commented May 30, 2018

@ashwoods What test would show it?

@Kentzo Kentzo closed this Aug 15, 2018
@Kentzo Kentzo reopened this Aug 15, 2018
@Kentzo
Copy link
Author

Kentzo commented Aug 15, 2018

Re-opened to re-run CI tests.

@ashwoods Any thoughts?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants