Should Access-Control-Allow-Origin header added to response When Exception Occur? #2876
-
[Problem Description]In the starlette version 0.45.4, When an unhandled exception (i.e., Exception with a 500 status code) occurs in the you can reproduce this: from starlette.applications import Starlette
from starlette.middleware.cors import CORSMiddleware
from starlette.requests import Request
from starlette.responses import Response, JSONResponse
from starlette.exceptions import HTTPException
from starlette import status
from starlette.routing import Route
class MyException(Exception):
def __init__(self, *args):
super().__init__(*args)
async def health(request):
return JSONResponse(status_code=status.HTTP_200_OK,content={"message": "healthy"})
async def throws_exception(request):
raise Exception("Exception")
async def throws_my_exception():
raise MyException("MyException!")
async def throws_http_exception(request):
raise HTTPException(
status_code=status.HTTP_405_METHOD_NOT_ALLOWED,
detail="HTTPException"
)
app = Starlette(
routes=[
Route("/health", health),
Route("/exception", throws_exception),
Route("/exception/my", throws_my_exception),
Route("/exception/http", throws_http_exception)
]
)
app.add_middleware(
CORSMiddleware, allow_origins=["*"], allow_credentials=False, allow_methods=["*"], allow_headers=["*"]
)
@app.exception_handler(MyException)
async def handle_super_exception(req: Request, exc: MyException):
# any status_code
return Response(status_code=status.HTTP_402_PAYMENT_REQUIRED,content="MyException Handled")
@app.exception_handler(Exception)
async def handle_general_exception(req: Request, exc: Exception):
return Response(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,content="Exception Handled") curl -i -X GET -H "Origin: https://google.com" http://127.0.0.1:8000/exception # no Access-Control-Allow-Origin header
curl -i -X GET -H "Origin: https://google.com" http://127.0.0.1:8000/exception/my # no Access-Control-Allow-Origin header
curl -i -X GET -H "Origin: https://google.com" http://127.0.0.1:8000/exception/http # Access-Control-Allow-Origin exist [Why This Is a Problem]From a developer’s perspective, it is confusing and unexpected that adding When handling requests from browser-based clients, unhandled exceptions should still comply with CORS requirements. The absence of the [Discussed Solutions]Previous discussions on this issue have taken place in Starlette’s Issue #617 and in FastAPI’s Discussion. The proposed solutions in those discussions include: 1. Wrapping the Starlette Application in CORSMiddleware: 2. Injecting CORS Headers at the ExceptionHandler Level: Need for a New Approach: In my view, CORS settings should apply globally. Relying on ExceptionHandlers to inject CORS headers in error responses feels inconsistent with the idea of global CORS policy enforcement. [Technical Root Cause]The technical issue arises from the order in which the middleware chain is constructed during the initialization of the [My Proposal]Option 1: Codebase Modification # starlette/applications.py, line 79
def build_middleware_stack(self) -> ASGIApp:
debug = self.debug
error_handler = None
exception_handlers: dict[typing.Any, typing.Callable[[Request, Exception], Response]] = {}
for key, value in self.exception_handlers.items():
if key in (500, Exception):
error_handler = value
else:
exception_handlers[key] = value
# If CORSMiddleware is registered, add the (500, Exception) handler to ExceptionMiddleware as well.
for middleware_to_stack in self.user_middleware:
# Ensure proper type checking; here we assume CORSMiddleware is imported from starlette.middleware.cors
if isinstance(middleware_to_stack.cls, CORSMiddleware):
exception_handlers[Exception] = error_handler
middleware = (
[Middleware(ServerErrorMiddleware, handler=error_handler, debug=debug)]
+ self.user_middleware
+ [Middleware(ExceptionMiddleware, handlers=exception_handlers, debug=debug)]
)
app = self.router
for cls, args, kwargs in reversed(middleware):
app = cls(app, *args, **kwargs)
return app Option 2: Official Documentation Tip Call for Feedback I am eager to submit a PR addressing this issue, but I would appreciate hearing other opinions and verifying that I haven't overlooked any important details. |
Beta Was this translation helpful? Give feedback.
Replies: 2 comments 1 reply
-
Additional Considerations for Option 1 For more detail, the Option 1 code is not perfect—it will produce an error if |
Beta Was this translation helpful? Give feedback.
-
Wrapping the app with the cors middleware is the official recommendation. PR welcome to improve docs. |
Beta Was this translation helpful? Give feedback.
Wrapping the app with the cors middleware is the official recommendation. PR welcome to improve docs.