-
-
Notifications
You must be signed in to change notification settings - Fork 128
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
Allow option to let unhandled Exceptions propagate through the adapter #285
Comments
@jb3rndt I recall some similar discussions in the past and that there may have been issue unrelated to the exception handling, but don't remember the details. Could you post a minimal example that demonstrates your use-case? |
Hi, yes here is a minimal example: from fastapi import FastAPI
from mangum import Mangum
app = FastAPI()
@app.get("/")
def test_route():
raise Exception("This is a test exception")
def handler(event, context):
try:
return Mangum(app)(event, context)
except Exception as e:
# Custom exception handling in the app root for all unhandled exceptions (not executed in Lambda)
print(repr(e))
event = {
"resource": "/",
"path": "/",
"httpMethod": "GET",
"headers": {
"accept": "text/html",
},
"queryStringParameters": None,
"multiValueQueryStringParameters": None,
"pathParameters": None,
"stageVariables": None,
"requestContext": {
"resourceId": "2gxmpl",
"resourcePath": "/",
"httpMethod": "GET",
"extendedRequestId": "JJbxmplHYosFVYQ=",
"requestTime": "10/Mar/2020:00:03:59 +0000",
"path": "/Prod/",
"accountId": "123456789012",
"protocol": "HTTP/1.1",
"stage": "Prod",
"domainPrefix": "70ixmpl4fl",
"requestTimeEpoch": 1583798639428,
"requestId": "77375676-xmpl-4b79-853a-f982474efe18",
"domainName": "70ixmpl4fl.execute-api.us-east-2.amazonaws.com",
"apiId": "70ixmpl4fl"
},
"body": None,
"isBase64Encoded": False
}
if __name__ == "__main__":
handler(event, None) |
Hi @jb3rndt why don't you try to use HTTPException from FastAPI , otherwise this part of the code can be changed like this
On the other hand if you need to retrieve the event and the context you can use starlette as follows.
I hope I have helped. |
I cant use HTTPException as the exception handler type because I want to catch all kinds of errors that occur during runtime to log them (with context information that I require) and reraise them afterwards. So I could use your suggestion with Apart from that, some problems still remain:
So without |
Any updates on this? |
I am having the same issue, @jb3rndt did you find a possible solution? |
Yes, I just reraise the exception. Check out this fork for the fix: https://github.com/jb3rndt/mangum. Codeimport asyncio
from contextlib import ExitStack
from typing import Type
from mangum import Mangum
from mangum.types import *
from mangum.protocols import LifespanCycle, HTTPCycle
from mangum.protocols.http import HTTPCycleState
class Magnum(Mangum):
def __init__(
self,
app: ASGI,
lifespan: LifespanMode = "auto",
api_gateway_base_path: str = "/",
custom_handlers: Optional[List[Type[LambdaHandler]]] = None,
text_mime_types: Optional[List[str]] = None,
exclude_headers: Optional[List[str]] = None,
) -> None:
super().__init__(app=app, lifespan=lifespan, api_gateway_base_path=api_gateway_base_path, custom_handlers=custom_handlers, text_mime_types=text_mime_types)
def __call__(self, event: LambdaEvent, context: LambdaContext) -> dict:
handler = self.infer(event, context)
with ExitStack() as stack:
if self.lifespan in ("auto", "on"):
lifespan_cycle = LifespanCycle(self.app, self.lifespan)
stack.enter_context(lifespan_cycle)
http_cycle = MagnumHTTPCycle(handler.scope, handler.body)
http_response = http_cycle(self.app)
return handler(http_response)
assert False, "unreachable" # pragma: no cover
class MagnumHTTPCycle(HTTPCycle):
def __init__(self, scope: Scope, body: bytes) -> None:
super().__init__(scope, body)
def __call__(self, app: ASGI) -> Response:
asgi_instance = self.run(app)
loop = asyncio.get_event_loop()
asgi_task = loop.create_task(asgi_instance)
loop.run_until_complete(asgi_task)
return {
"status": self.status,
"headers": self.headers,
"body": self.body,
}
async def run(self, app: ASGI) -> None:
try:
await app(self.scope, self.receive, self.send)
except BaseException as exc:
if self.state is HTTPCycleState.REQUEST:
await self.send(
{
"type": "http.response.start",
"status": 500,
"headers": [[b"content-type", b"text/plain; charset=utf-8"]],
}
)
await self.send(
{
"type": "http.response.body",
"body": b"Internal Server Error",
"more_body": False,
}
)
elif self.state is not HTTPCycleState.COMPLETE:
self.status = 500
self.body = b"Internal Server Error"
self.headers = [[b"content-type", b"text/plain; charset=utf-8"]]
raise exc from None |
@jb3rndt this is looking appropriate! (And I also landed here because of wanting Cloudwatch errors metric to be truthful for a lambda.) If your approach was sufficiently battle tested maybe there's a PR you'd be keen to spawn from your fork? |
I am not sure what would be required to have this approach "battle tested" but I am happy to file a PR for that. Although I am unsure whether the maintainer(s) consider this change to be important? So before putting more work into it, it would be nice to know if this is desired :) |
Hi, thank you for this nice tool to wrap asgi applications. It makes deploying an API to lambda very easy while using a server to emulate it locally.
However, I found out that, in case of unhandled runtime errors in my code, the adapter automatically returns a json with a status code of 500. While that is totally fine, the original error gets caught in the
HTTPCycle.run
method which makes a manual handling of all uncaught errors impossible (afaik). I want to let the error propagate to the lambda execution runtime, so that the Cloudwatch "ERROR" metric for the function recognizes the execution as a failure and thus the alarms are triggered.The way it is right now, all errors are just caught and not even aws knows that something unexpected happened.
Is there a way to fix that or something essential I am missing? One solution could be to optionally reraise all exceptions in the mentioned method.
Thanks 👋
The text was updated successfully, but these errors were encountered: