collapse_excgroups context manager removing exception context

[ccrims0n]

collapse_excgroups used in the BaseHTTPMiddleware has the following code:

try:
    yield
except BaseException as exc:
    if has_exceptiongroups:
        while isinstance(exc, BaseExceptionGroup) and len(exc.exceptions) == 1:
            exc = exc.exceptions[0]  # pragma: no cover

    raise exc from None

The last line of this function updates the __context__ of the exception object, exc. This also adds unnecessary context Another exception happened during the handling of exception.

I am not sure if this is by design but this is unnecessary and can obscure details of how the exception was raised.

Fix
Both the cases, exception group object and the exception object should be handled separately. Only raise should be used to re-raise caught exception. For the exception group, the right way needs to be figured out.

[jurasofish]

You can see this with the example below: ExcOuter is totally lost in the final traceback. Starlette just eats it - this is a big deal because if code in an app's internal exception hander fails, the original exception details are totally lost.

If you fiddle collapse_excgroups to raise exc from exc.__cause__ or exc.__context__ and middleware call_next to raise app_exc from app_exc.__cause__ or app_exc.__context__ it is possible to get the details to propagate through - but it is actually changing the exceptions from how they really should be so not great.

I've had a dig and I'm not sure what a good solution here is. Like if you have something like app_exc: Exception as a variable, and you're no longer in the except block that handled it, and you want to re-raise it from another except block (like a control flow exception handler like except anyio.EndOfStream) then it's quite hard to ensure that the re-raised exception does not have its context/cause replaced by that of the final except block's exception.

Exception groups seem to handle this quite gracefully though, like if I try except anyio.EndOfStream: ... raise ExceptionGroup("Error in app", [app_exc]) and remove use of collapse_excgroups then I do get full exception details in the output (though quite chaotic). Questionable given starlette's support for python versions <3.11 though. I suppose the exception group backport lib is already a dependency so maybe it could be used. Or maybe code could conditionally raise an exception group for >=3.11 and continue with current behaviour for <3.11

import traceback
from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
from starlette.requests import Request
from starlette.responses import Response
from starlette.routing import Route
from starlette.testclient import TestClient


def exc_with_context(request: Request) -> None:
    try:
        raise Exception("ExcOuter")
    except Exception as e:

        # E.g. maybe someone does `log("uh oh!" + e)` and that results in a new
        # exception due to bad logging. That is, explicit exception chaining
        # is NOT a practical option here.

        # ALso worth checking explicit chaining like `raise Exception("Blah") from e
        raise Exception("ExcInner")

class CustomMiddleware(BaseHTTPMiddleware):
    async def dispatch(
        self,
        request: Request,
        call_next: RequestResponseEndpoint,
    ) -> Response:

        try:
            response = await call_next(request)
        except Exception:
            # print("----start inside middleware----")
            # traceback.print_exc()
            # print("----end inside middleware----")
            raise
            # exc_fmt = "".join(traceback.format_exception(ctx.value))  # type: ignore[call-arg,arg-type]
            # print(exc_fmt)
        return response

app = Starlette(
    routes=[Route("/exc-with-context", endpoint=exc_with_context)],
    middleware=[Middleware(CustomMiddleware)],
)

client = TestClient(app)
client.get("/exc-with-context")