Exceptions raised in middleware handled by differently

[hongymagic]

Description

It seems that various of middlewares behave differently when it comes to exception handling:

• Starlette's AuthenticationMiddleware has its own error handler (to return responses)
• Starlette's BaseHTTPMiddleware always returns 500 even if you throw HTTPException

So for example, if you were to turn FastAPI's basic authentication into middleware instead of it being per-route operation, none of the errors thrown in HTTPBasic.__call__ are respected and just return 500 internal server error.

In frameworks like Express.js or Koa, you can either:

1. Rely on ordering of the middleware, and by having error handlers at the top of the middleware chain - capture exceptions and have consistent responses; or
2. Have application level exception handling for most things, expecially routes and middleware.

I guess the first one is the more relevant approach here.

I've not looked too deep in to the code base, but the behaviours seems a bit odd at first.

[dmontagu]

The reason for this seems to be that the add_middleware method inherited from Starlette adds the middleware to the error_middleware, rather than the exception_middleware. This means that the exception_middleware (which does things like convert HTTPException to neatly formatted 400s) actually ends up at the very inside of the onion.

Your issues all seem to have their source in functionality inherited directly from starlette, so this should probably be brought up as an issue in starlette. (I suspect that if you were to bring this up in starlette, the response would be that middlewares should be responsible for handling their own exceptions and returning some sort of starlette.Response rather than raising a handled exception, and that exceptions in middleware really should be 500s. But I think it's worth creating a starlette issue if you think this is the right approach; maybe the response would surprise me 😅 .)

---

Separately, if you don't like this, you could just override the add_middleware method on a custom FastAPI subclass, and change it to add the middleware to the exception_middleware, so that middleware will sit on the outside of the onion and handle errors even inside the middleware.

[hongymagic]

that exceptions in middleware really should be 500s

That is definitely not the case with their AuthenticationMiddleware, and even with errors - no reason to ditch the standard "error response" models we use..

But they certainly do handle their own errors, esp in the middleware mentioned above. We can start with our own base middleware which handles error responses like exception handlers - but it does suck that you've gotta handle it in two different places to get consistent output.

Will close this one.

The previous issue #457 is even more annoying, because we're just trying to use a standard CORSHandler with standard exception handler - now we've gotta intervene to fix it. And I can't find a good reason why anyone using FastAPI and/or starlette to want that behaviour.. Will leave that one open.

[dmontagu]

@hongymagic

To clarify, the reason I said middleware errors should be 500s above was that any expected errors should be handled directly in the middleware, and that errors initiating from a given middleware without being gracefully handled inside the middleware would presumably be implementation mistakes (worthy of a 500 response).

I think you made some good points here, and, for example, currently I don't think there is an excellent pattern for auth-in-middleware (for the reasons you described). It seems to me like there are (at least) three possible solutions to the auth-in-middleware problem that could result in usability improvements for FastAPI:

1. A better API for adding middleware to the inside of the middleware onion (i.e., inside the exception_middleware, rather than just inside the error_middleware).
   • This might be better suited as a starlette feature request
2. The ability to reference dependencies added to the router from inside an endpoint call
   • This would enable you to move more things outside of middleware without needing to repeat yourself in every route definition, but:
   • Would probably result in an awkward, difficult-to-type-check API
3. A better form of accessible dependency reuse, e.g., via class-based views
   • This could reduce the amount of repetition, and would enable a type-safe API, but:
   • Would amount to a substantial new feature for fastapi, and would may add a higher cognitive burden than the current highly-intuitive endpoint APIs

It might be worth thinking about which, if any, of the above seems most useful to you, and opening an issue with a feature request in fastapi/starlette as appropriate.

[tiangolo]

Thanks for your help here as always @dmontagu ! 🍰🚀

@hongymagic let me ask you to be less pejorative against the project and its community. We are putting our free time here to help you.

---

It seems you're expecting some things to behave in a predefined way with a previous and subjective concept of what is a "standard" middleware design and behavior.

If you use the middlewares from Starlette for auth, they will behave in the way it's designed in Starlette.

If you "turn FastAPI's basic authentication into middleware instead of it being per-route operation" you're not using FastAPI's basic auth ideas, you're creating a new middleware that works the way it does in Starlette. If you want to declare those things in a single place, your might want to use a dependency in a include_router.

[hongymagic]

let me ask you to be less pejorative against the project and its community. We are putting our free time here to help you.

Apologies, definitely did not mean to be that way, and I do appreciate the time spend answering questions.

---

You are right my understanding of middleware when I read the docs (and as I've cited before) come from other places and make same assumptions about Starlette/FastAPI - when it shouldn't. Thanks for your time, and appreciate the work you guys are doing on FastAPI!

[lagunovartur]

Hello, i have similar problem:

from typing import Callable

from fastapi import Request, Response
from starlette.middleware.base import BaseHTTPMiddleware

from fr_common_lib.connections.db import db
from app.core.context import ContextManager
from app.schemas.req_context import RequestContext
from app.services import jwt


class ContextMW(BaseHTTPMiddleware):

    async def dispatch(self, request: Request, call_next: Callable, **kwargs) -> Response:

        customer_id = None
        token_payload = None

        access_token = request.cookies.get("X-Access-Token", None)
        if access_token:
            try:
                token_payload = jwt.token_payload(access_token)
                customer_id = token_payload.cid
            except Exception as exc:
                token_payload = exc

        db_schema = f't{customer_id}' if customer_id else 'tenant'
        async with db.async_session(db_schema) as db_session:

            req_context = RequestContext(
                request=request,
                token_payload=token_payload,
                db_session=db_session
            )

            ContextManager.set_context(req_context)
            response = await call_next(request)

        return response

    @asynccontextmanager
    async def async_session(self, schema: str, **kwargs) -> AsyncContextManager[AsyncSession]:

        connectable = self._async_engine.execution_options(schema_translate_map=dict(tenant=schema))
        session = self._async_session_factory(bind=connectable, **kwargs)

        try:
            yield session
            await session.commit()
        except Exception as exc:
            await session.rollback()
            raise
        finally:
            await session.close()

async def sqla_err_handler(request, exc):
    return JSONResponse(
        status_code=400,
        content={
            "message": "Произошла ошибка базы данных",
            "detail": str(exc)
        }
    )

app.add_exception_handler(SQLAlchemyError, sqla_err_handler)

as result, i have error 500, instead 400 in global error handler, because, middleware context manager, raise error again