Skip to content

Conversation

danlapid
Copy link
Contributor

@danlapid danlapid commented Apr 13, 2025

Summary

In https://github.com/encode/starlette/blob/master/starlette/_exception_handler.py#L61 we check if the exception_handler is an async function and if not we execute it in a different thread.
This is meant to ensure that long-running sync IO operations don't block the main async eventloop.
However, in the case of http_exception, a simple function that does not do any io at all - the aforementioned logic spawns a thread to run the function.
Marking this function async is a simple way of making sure this function does not accidently cause a thread to be created.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@danlapid danlapid force-pushed the danlapid/mark_http_exception_handler_as_async branch 5 times, most recently from bdd5b6b to 117ec49 Compare April 13, 2025 22:31
…eation.

In https://github.com/encode/starlette/blob/master/starlette/_exception_handler.py#L61 we check if the exception_handler is an async function and if not we execute it in a different thread.
This is meant to ensure that long-running sync IO operations don't block the main async eventloop.
However, in the case of http_exception, a simple function that does not do any io at all the aforementioned logic spawns a thread to run the function.
Marking this function async is a simple way of making sure this function does not accidently cause a thread to be created.
@danlapid danlapid force-pushed the danlapid/mark_http_exception_handler_as_async branch from 117ec49 to d17b116 Compare April 13, 2025 22:33
Copy link
Collaborator

@Kludex Kludex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :)

I'm not sure how no one saw this...

@danlapid
Copy link
Contributor Author

@Kludex no worries 😄
Do you mind hitting the merge button as well? I don't have permissions 🙏

@danlapid
Copy link
Contributor Author

@Kludex bumping this, would appreciate some help hitting the merge button 😄

@encode encode locked as spam and limited conversation to collaborators Apr 16, 2025
@adriangb adriangb merged commit a766a58 into encode:master Apr 16, 2025
7 checks passed
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