Skip to content

🐛 Fix bug overriding custom HTTPException and RequestValidationError from exception_handlers#1924

Merged
tiangolo merged 1 commit intofastapi:masterfrom
uriyyo:fix-exception-handlers-overriding
Nov 5, 2020
Merged

🐛 Fix bug overriding custom HTTPException and RequestValidationError from exception_handlers#1924
tiangolo merged 1 commit intofastapi:masterfrom
uriyyo:fix-exception-handlers-overriding

Conversation

@uriyyo
Copy link
Contributor

@uriyyo uriyyo commented Aug 19, 2020

Fix issue/related to #1440.

A fix is simple, set HTTPException and RequestValidationError exception handlers only in case when they don't set.

@codecov
Copy link

codecov bot commented Aug 19, 2020

Codecov Report

Merging #1924 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1924   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          239       240    +1     
  Lines         7079      7102   +23     
=========================================
+ Hits          7079      7102   +23     
Impacted Files Coverage Δ
fastapi/applications.py 100.00% <100.00%> (ø)
tests/test_exception_handlers.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a689796...391fba2. Read the comment docs.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 391fba2 at: https://5f3d6c9962cbd41e7c6cc084--fastapi.netlify.app

@uriyyo
Copy link
Contributor Author

uriyyo commented Aug 20, 2020

This PR solve same issue as #1887 solve, I didn't see it because they point to two different issues related to the same problem.

Basically my idea was to reduce the number of calls to Starlette.build_middleware_stack method.

Previously there can be 3 calls to this method because in FastAPI.setup will call FastAPI.add_exception_handler method which will call Starlette.build_middleware_stack.

With this bugfix, there will be always only one call to the Starlette.build_middleware_stack method.

@tiangolo tiangolo changed the title Fix bug when HTTPException and RequestValidationError exception can't be overridden using exception_handlers 🐛 Fix bug overriding custom HTTPException and RequestValidationError from exception_handlers Nov 5, 2020
@tiangolo tiangolo merged commit 4ce1816 into fastapi:master Nov 5, 2020
@tiangolo
Copy link
Member

tiangolo commented Nov 5, 2020

Awesome! Great job @uriyyo ! ☕ 🍰

Thanks for pointing to the previous PR. I ended up merging this one as the code was slightly simpler 🤓

This is included in FastAPI 0.61.2 (released in some minutes).

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