Skip to content

Commit

Permalink
Changed exception handlers to always run, but render response only if…
Browse files Browse the repository at this point in the history
… it's yet to be rendered
  • Loading branch information
Aviram Hassan committed Dec 18, 2019
1 parent c096935 commit fade736
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
9 changes: 4 additions & 5 deletions starlette/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,15 @@ async def sender(message: Message) -> None:
if handler is None:
raise exc from None

if response_started:
msg = "Caught handled exception, but response already started."
raise RuntimeError(msg) from exc

request = Request(scope, receive=receive)
if asyncio.iscoroutinefunction(handler):
response = await handler(request, exc)
else:
response = await run_in_threadpool(handler, request, exc)
await response(scope, receive, sender)

if not response_started:
# Send the response only if not response started already.
await response(scope, receive, sender)

def http_exception(self, request: Request, exc: HTTPException) -> Response:
if exc.status_code in {204, 304}:
Expand Down
21 changes: 18 additions & 3 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,26 @@ def test_websockets_should_raise():


def test_handled_exc_after_response():
# A 406 HttpException is raised *after* the response has already been sent.
# The exception middleware should raise a RuntimeError.
with pytest.raises(RuntimeError):
wrapper_used = False

def handler_wrapper(func):
def wrapper(*args, **kwargs):
nonlocal wrapper_used
wrapper_used = True
return func(*args, **kwargs)

return wrapper

try: # Revert state no matter what happens
key, func = app._exception_handlers.popitem()
app._exception_handlers[key] = handler_wrapper(func)

client.get("/handled_exc_after_response")

assert wrapper_used
finally:
app._exception_handlers[key] = func

# If `raise_server_exceptions=False` then the test client will still allow
# us to see the response as it will have been seen by the client.
allow_200_client = TestClient(app, raise_server_exceptions=False)
Expand Down

0 comments on commit fade736

Please sign in to comment.