Skip to content

Allow async exception handlers to type-check #2949

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

Merged
merged 7 commits into from
Jun 13, 2025

Conversation

jonathanberthias
Copy link
Contributor

Summary

There has been a lot of discussion around the exception handlers' type annotations, e.g., #2403 and #2048, but there is a simple case that isn't currently handled and can be fixed easily.

When building the ExceptionMiddleware directly, only sync handlers are accepted, even though the code can perfectly well handle async handlers. I think this was not noticed before because not many users instantiate this middleware directly, and when they do, the errors linked to the exceptions' variance always led to adding # type: ignores, missing this simple error.

The fix is just to reuse the existing ExceptionHandler type alias.

I wasn't sure how to add tests for these changes. I added some code in test_exceptions.py that is just used to check there is not type error, let me know if there is another preferred approach.

While using the ExceptionHandler alias in a few places, it unearthed another error that can be solved by using TypeIs instead of TypeGuard in is_async_callable. This fixes a few false positives from MyPy, though is not directly related to the initial change in this PR so I totally understand if you prefer I leave it out.

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.

@jonathanberthias jonathanberthias requested a review from Kludex June 3, 2025 12:54
@Kludex
Copy link
Member

Kludex commented Jun 13, 2025

Thanks! 🙏

@Kludex Kludex merged commit 739ea49 into encode:master Jun 13, 2025
7 checks passed
@jonathanberthias jonathanberthias deleted the async-exception-handlers branch June 13, 2025 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants