fastapi/fastapiPublic

NotificationsYou must be signed in to change notification settings
Fork8.7k
Star95.4k

🐛Fix`RuntimeError` raised when`HTTPException` has a status code with no content#5365

Merged

tiangolo merged 8 commits intofastapi:masterfrom

iudeen:fix/http-exception-no-content-error

Sep 11, 2022

Merged

🐛Fix`RuntimeError` raised when`HTTPException` has a status code with no content#5365
tiangolo merged 8 commits intofastapi:masterfrom
iudeen:fix/http-exception-no-content-error

Conversation

Copy link

Contributor

iudeen commentedSep 8, 2022•
edited by tiangolo
Loading

Fixes/related to#4949 (comment)

Please advice if this approach is correct one.

Suggestions welcome!

fix: handle no body status codes in HTTPExceptions

afecf9b

Copy link

codecovbot commentedSep 8, 2022•
edited
Loading

Codecov Report

Merging#5365 (1fe25cf) intomaster (6620273) willnot change coverage.
The diff coverage is100.00%.

@@            Coverage Diff            @@##            master     #5365   +/-   ##=========================================  Coverage   100.00%   100.00%           =========================================  Files          540       540             Lines        13936     13951   +15     =========================================+ Hits         13936     13951   +15

Impacted Files	Coverage Δ
fastapi/exception_handlers.py	`100.00% <100.00%> (ø)`
tests/test_starlette_exception.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell ushow you rate us. Have a feature suggestion?Share it here.

Copy link

Contributor

github-actionsbot commentedSep 8, 2022

📝 Docs preview for commitafecf9b at:https://631a48f65533fb0233157a85--fastapi.netlify.app

fix: added tests for No Body Status Code Exception Handlers

f96a078

Copy link

Contributor

github-actionsbot commentedSep 8, 2022

📝 Docs preview for commitf96a078 at:https://631a4cce339597069b76e096--fastapi.netlify.app

iudeen added2 commits

September 9, 2022 01:45

fix: added schema for new tests to openapi json

3c79e5b

fix: added schema for new tests to openapi json

c357017

Copy link

Contributor

github-actionsbot commentedSep 8, 2022

📝 Docs preview for commitc357017 at:https://631a4e7d882b77006ea53e5b--fastapi.netlify.app

iudeen changed the title~~fix: handle no body status codes in HTTPExceptions~~ 🐛Fix Handle HTTPExceptions with status codes that should not support body

Sep 8, 2022

iudeen changed the title ~~🐛Fix Handle HTTPExceptions with status codes that should not support body~~ 🐛Fix RuntimeError that got raised when HTTPExceptions with status codes that should not support body was raised

Sep 8, 2022

Kludex approved these changes

Sep 8, 2022

View reviewed changes

Copy link

Member

Kludex left a comment

Choose a reason for hiding this comment

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

Yep, same idea as what we have in Starlette:https://github.com/encode/starlette/blob/bc61505faef15b673bf4bf65db4927daa354d6b8/starlette/middleware/exceptions.py#L99-L104

tests/test_starlette_exception.py OutdatedShow resolvedHide resolved

Update tests/test_starlette_exception.py

f26af44

Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>

Copy link

ContributorAuthor

iudeen commentedSep 8, 2022•
edited
Loading

Yep, same idea as what we have in Starlette:https://github.com/encode/starlette/blob/bc61505faef15b673bf4bf65db4927daa354d6b8/starlette/middleware/exceptions.py#L99-L104

I was able to find the bug by adding debug points toExceptionMiddleware while running the app usingFastAPI andStarlette in parallel.

And thus, the code was referred from Starlette! (Sorry, should have mentioned)

Copy link

Contributor

github-actionsbot commentedSep 8, 2022

📝 Docs preview for commitf26af44 at:https://631a5fedd66a3a17d38dcc68--fastapi.netlify.app

fix: used common headers

fda6d88

Copy link

Contributor

github-actionsbot commentedSep 9, 2022

📝 Docs preview for commitfda6d88 at:https://631a9d31d66a3a47438dc97b--fastapi.netlify.app

BilalAlpaslan approved these changes

Sep 9, 2022

View reviewed changes

JarroVGIT approved these changes

Sep 9, 2022

View reviewed changes

Copy link

Contributor

JarroVGIT left a comment

Choose a reason for hiding this comment

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

LGTM

tiangolo added2 commits

September 11, 2022 18:09

🎨 Format text in OpenAPI test

d21ce64

♻️ Simplify exception handler implementation

1fe25cf

tiangolo changed the title ~~🐛Fix RuntimeError that got raised when HTTPExceptions with status codes that should not support body was raised~~ 🐛FixRuntimeError raised whenHTTPException has a status code with no content

Sep 11, 2022