-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
drop error_response, add error detail and support for serialization #59
base: master
Are you sure you want to change the base?
Changes from all commits
48f594d
6e2fdd6
37cafcf
7e68f20
330ad2d
c7fa659
fa3f1c6
acd4191
8084142
9076553
ca5cc02
20e79f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,36 +1,44 @@ | ||
from starlette.exceptions import HTTPException | ||
from typing import Optional | ||
from starlette.responses import Response | ||
from starlette import status | ||
|
||
|
||
class StarletteContextError(BaseException): | ||
pass | ||
class StarletteContextException(HTTPException): | ||
status_code = status.HTTP_400_BAD_REQUEST | ||
detail = "Error" | ||
|
||
def __init__( | ||
self, | ||
detail: Optional[str] = None, | ||
status_code: Optional[int] = None, | ||
) -> None: | ||
super().__init__( | ||
status_code=status_code or self.status_code, | ||
detail=detail or self.detail, | ||
) | ||
|
||
|
||
class ContextDoesNotExistError(RuntimeError, StarletteContextError): | ||
def __init__(self): | ||
self.message = ( | ||
class ContextDoesNotExistError(StarletteContextException): | ||
status_code = status.HTTP_500_INTERNAL_SERVER_ERROR | ||
|
||
def __str__(self): # pragma: no cover | ||
return ( | ||
"You didn't use the required middleware or " | ||
"you're trying to access `context` object " | ||
"outside of the request-response cycle." | ||
"outside of the request-response cycle" | ||
) | ||
super().__init__(self.message) | ||
|
||
|
||
class ConfigurationError(StarletteContextError): | ||
pass | ||
|
||
class ConfigurationError(StarletteContextException): | ||
status_code = status.HTTP_500_INTERNAL_SERVER_ERROR | ||
|
||
class MiddleWareValidationError(StarletteContextError): | ||
def __init__( | ||
self, *args, error_response: Optional[Response] = None | ||
) -> None: | ||
super().__init__(*args) | ||
self.error_response = error_response | ||
def __str__(self): # pragma: no cover | ||
return "Invalid starlette-context configuration" | ||
|
||
|
||
class WrongUUIDError(MiddleWareValidationError): | ||
pass | ||
class WrongUUIDError(StarletteContextException): | ||
detail = "Invalid UUID in request header" | ||
|
||
|
||
class DateFormatError(MiddleWareValidationError): | ||
pass | ||
class DateFormatError(StarletteContextException): | ||
detail = "Date header in wrong format, has to match rfc1123" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import logging | ||
from typing import Awaitable, Callable, Optional, Union | ||
|
||
from starlette_context.errors import StarletteContextException | ||
from starlette.responses import PlainTextResponse, Response | ||
from starlette.requests import Request | ||
|
||
|
||
class StarletteContextMiddlewareMixin: | ||
def __init__( | ||
self, | ||
error_handler: Optional[ | ||
Callable[ | ||
[Request, StarletteContextException], | ||
Union[Response, Awaitable[Response]], | ||
] | ||
] = None, | ||
log_errors: bool = False, | ||
*args, | ||
**kwargs, | ||
): | ||
self.error_handler = error_handler or self.default_error_handler | ||
self.log_errors = log_errors | ||
|
||
async def log_error( | ||
self, request: Request, exc: StarletteContextException | ||
) -> None: | ||
if self.log_errors: | ||
logger = self.get_logger() | ||
logger.exception("starlette-context exception") | ||
hhamana marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@staticmethod | ||
async def default_error_handler( | ||
request: Request, exc: StarletteContextException | ||
) -> Response: | ||
return PlainTextResponse( | ||
content=exc.detail, status_code=exc.status_code | ||
) | ||
|
||
@staticmethod | ||
def get_logger(): | ||
return logging.getLogger("starlette_context") # pragma: no cover | ||
|
||
async def create_response_from_exception( | ||
self, request: Request, exc: StarletteContextException | ||
) -> Response: | ||
""" | ||
Middleware / plugins exceptions will always result in 500 plain text | ||
errors. | ||
|
||
These errors can't be handled using exception handlers passed to the | ||
app instance / middleware. If you need to customize the response, | ||
handle it here. | ||
""" | ||
await self.log_error(request, exc) | ||
return await self.error_handler(request, exc) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,24 +2,25 @@ | |
from typing import Optional, Sequence, Union | ||
|
||
from starlette.requests import HTTPConnection, Request | ||
from starlette.responses import Response | ||
from starlette.types import ASGIApp, Message, Receive, Scope, Send | ||
|
||
from starlette_context import _request_scope_context_storage | ||
from starlette_context.middleware.mixin import StarletteContextMiddlewareMixin | ||
from starlette_context.plugins import Plugin | ||
from starlette_context.errors import ( | ||
ConfigurationError, | ||
MiddleWareValidationError, | ||
StarletteContextException, | ||
) | ||
|
||
|
||
class RawContextMiddleware: | ||
class RawContextMiddleware(StarletteContextMiddlewareMixin): | ||
def __init__( | ||
self, | ||
app: ASGIApp, | ||
plugins: Optional[Sequence[Plugin]] = None, | ||
default_error_response: Response = Response(status_code=400), | ||
**kwargs, | ||
) -> None: | ||
super().__init__(**kwargs) | ||
self.app = app | ||
for plugin in plugins or (): | ||
if not isinstance(plugin, Plugin): | ||
|
@@ -28,7 +29,6 @@ def __init__( | |
) | ||
|
||
self.plugins = plugins or () | ||
self.error_response = default_error_response | ||
|
||
async def set_context( | ||
self, request: Union[Request, HTTPConnection] | ||
|
@@ -70,29 +70,30 @@ async def send_wrapper(message: Message) -> None: | |
|
||
try: | ||
context = await self.set_context(request) | ||
except MiddleWareValidationError as e: | ||
if e.error_response is not None: | ||
error_response = e.error_response | ||
else: | ||
error_response = self.error_response | ||
token: Token = _request_scope_context_storage.set(context) | ||
|
||
message_head = { | ||
try: | ||
await self.app(scope, receive, send_wrapper) | ||
finally: | ||
_request_scope_context_storage.reset(token) | ||
|
||
except StarletteContextException as e: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moving the handling to wrap everything? no, that can't be right. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well I'm glad I didn't merge it yet. Thanks! |
||
response = await self.create_response_from_exception(request, e) | ||
|
||
message_head: Message = { | ||
"type": "http.response.start", | ||
"status": error_response.status_code, | ||
"headers": error_response.raw_headers, | ||
"status": response.status_code, | ||
"headers": [ | ||
(k.encode(), v.encode()) | ||
for k, v in response.headers.items() | ||
], | ||
} | ||
|
||
await send(message_head) | ||
|
||
message_body = { | ||
message_body: Message = { | ||
"type": "http.response.body", | ||
"body": error_response.body, | ||
"body": response.body, | ||
} | ||
await send(message_body) | ||
return | ||
|
||
token: Token = _request_scope_context_storage.set(context) | ||
|
||
try: | ||
await self.app(scope, receive, send_wrapper) | ||
finally: | ||
_request_scope_context_storage.reset(token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return await self.error_handler(request, exc)
that looks very much like only awaitables are supported. why the union?
(and i'd rather support only one than digging into the can of worms of supporting either)