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.

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

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

Merged
merged 5 commits into from Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion starlette/middleware/base.py
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
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