litestar: Middleware are not executed on Starlite error
When route is not found, method is not allowed, or some similar error, then middleware are not executed although and it is unexpected behaviour.
Starlite, second test fails:
from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
from starlette.requests import Request
from starlette.responses import Response
from starlite import Starlite, TestClient, get
class ServerHeaderMiddleware(BaseHTTPMiddleware):
async def dispatch(
self, request: Request, call_next: RequestResponseEndpoint
) -> Response:
response = await call_next(request)
response.headers["Server"] = "NULL"
return response
@get()
async def route() -> str:
return ""
app = Starlite(
debug=True,
route_handlers=[route],
middleware=[ServerHeaderMiddleware],
)
def test_middleware_is_executed_when_route_found():
with TestClient(app) as client:
response = client.get("/")
assert response.headers["Server"] == "NULL"
def test_middleware_is_executed_when_route_not_found():
with TestClient(app) as client:
response = client.get("/non-existing-route")
assert response.headers["Server"] == "NULL"
Starlette, tests pass:
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 PlainTextResponse, Response
from starlette.routing import Route
from starlette.testclient import TestClient
class ServerHeaderMiddleware(BaseHTTPMiddleware):
async def dispatch(
self, request: Request, call_next: RequestResponseEndpoint
) -> Response:
response = await call_next(request)
response.headers["Server"] = "NULL"
return response
async def route(request: Request):
return PlainTextResponse("")
app = Starlette(
debug=True,
routes=[Route("/", route, methods=["GET"])],
middleware=[Middleware(ServerHeaderMiddleware)],
)
def test_middleware_is_executed_when_route_found():
with TestClient(app) as client:
response = client.get("/")
assert response.headers["Server"] == "NULL"
def test_middleware_is_executed_when_route_not_found():
with TestClient(app) as client:
response = client.get("/non-existing-route")
assert response.headers["Server"] == "NULL"
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 66 (65 by maintainers)
Commits related to this issue
- Provides an impl. of exception handling middleware. For #175 — committed to litestar-org/litestar by peterschutt 2 years ago
- Provides an impl. of exception handling middleware. For #175 — committed to litestar-org/litestar by peterschutt 2 years ago
Sure, lets get started with rolling out the solution as you see it. It will help me to understand your proposal better and we can discuss as it evolves.
Not at all, you asked for my opinion and I’ve given it. I’m trying to conceptualize the problem and all my points and questions are purely aimed toward that. So far, you’ve not directly addressed any point I’ve made but have made it clear that you don’t agree, and that’s fine. Did you see this:
Sorry about the image 😆 That’s pretty much what I’m thinking. It would be nice if the response could find its way back to the traditional middleware stack after exiting “innerware” stack so it could be inspected/modified but I’d have to have a look to know if possible… could that be done?
Thanks for your effort to help with this issue and passion for the project, by the way! ❤️
Ok, we discussed it and for now we will wait to see if this is indeed an issue. CORS will run on these errors - because that happens outside of the middleware stack. We might add a lifecycle hook in the future that allows modifying these in the ASGI router. For now though we will keep behaviour as is.
Thanks for testing!
Well, I think what you’re proposing is viable, but it places some conceptual complexities for the user. From my PoV it’s better to abstract complexity into our end. Saying that, it needs to be discussed.
Would middleware run on exception then?
Sentry works - I use it already, just pass to middleware on the app 👍 there’s a typing issue with it though in pycharm but no biggy.
Well I’m just playing the hand that’s in-front of me - I probably won’t spend any more time on thinking about this for the rest of the day. Feel free to detail the issues with those two middlewares and some potential solutions.
ok, i left this out of my PR for now, we will deal with it separately once there is a clear plan on how to do this.