Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Prevent anyio.ExceptionGroup in error views under a BaseHTTPMiddleware #1262

Merged
merged 5 commits into from
Oct 28, 2021
Merged

Prevent anyio.ExceptionGroup in error views under a BaseHTTPMiddleware #1262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58defbc
Prevent ExceptionGroup in error views under a BaseHTTPMiddleware
florimondmanca Aug 11, 2021
ccb6e2a
Merge branch 'master' into fm/anyio-middleware-view-error
euri10 Oct 22, 2021
18a7209
Apply suggestion from @uSpike
florimondmanca Oct 22, 2021
34c1f35
Merge branch 'master' into fm/anyio-middleware-view-error
florimondmanca Oct 22, 2021
f0142df
Merge branch 'master' into fm/anyio-middleware-view-error
florimondmanca Oct 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion starlette/middleware/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,25 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
return

async def call_next(request: Request) -> Response:
app_exc: typing.Optional[Exception] = None
send_stream, recv_stream = anyio.create_memory_object_stream()

async def coro() -> None:
nonlocal app_exc

async with send_stream:
await self.app(scope, request.receive, send_stream.send)
try:
await self.app(scope, request.receive, send_stream.send)
except Exception as exc:
app_exc = exc

task_group.start_soon(coro)

try:
message = await recv_stream.receive()
except anyio.EndOfStream:
if app_exc is not None:
raise app_exc
raise RuntimeError("No response returned.")

assert message["type"] == "http.response.start"
Expand Down
5 changes: 3 additions & 2 deletions tests/middleware/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def homepage(request):

@app.route("/exc")
def exc(request):
raise Exception()
raise Exception("Exc")


@app.route("/no-response")
Expand All @@ -52,8 +52,9 @@ def test_custom_middleware(test_client_factory):
response = client.get("/")
assert response.headers["Custom-Header"] == "Example"

with pytest.raises(Exception):
with pytest.raises(Exception) as ctx:
Copy link
Member Author

Choose a reason for hiding this comment

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

When running on trio, previously we would be getting an unexpected "Portal is not running" error here, and misinterpreting it as the Exception() in the /exc view. So I made the latter more precise and made sure we're explicitly checking for it.

response = client.get("/exc")
assert str(ctx.value) == "Exc"

with pytest.raises(RuntimeError):
response = client.get("/no-response")
Expand Down