Skip to content
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

"raise exc from None" interferes with exception chaining #1114

Closed
2 tasks done
oTree-org opened this issue Dec 15, 2020 · 5 comments · Fixed by #1158
Closed
2 tasks done

"raise exc from None" interferes with exception chaining #1114

oTree-org opened this issue Dec 15, 2020 · 5 comments · Fixed by #1158

Comments

@oTree-org
Copy link
Contributor

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

Even though I use exception chaining (raise ... from) in my app, the original exception info gets deleted because Starlette handles exceptions with raise .. from None at the outer scope.
Here is a MWE:

from starlette.applications import Starlette
from starlette.endpoints import HTTPEndpoint
from starlette.responses import HTMLResponse
from starlette.routing import Route


class TemplateRenderingError(Exception): pass


class Homepage(HTTPEndpoint):
    async def get(self, request):
        try:
            content = '{a}'.format()
        except KeyError as e:
            raise TemplateRenderingError from e
        return HTMLResponse(content)

app = Starlette(debug=True, routes=[
    Route('/', Homepage),
])

Run the server, load /, then look at the console output.

What I expect to get is a chained exception:

ERROR: Exception in ASGI application
Traceback (most recent call last):
  File ".\star_bug.py", line 13, in get
    content = '{a}'.format()
KeyError: 'a'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "c:\otree\ve-nodj\lib\site-packages\uvicorn\protocols\http\h11_impl.py", line 394, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "c:\otree\ve-nodj\lib\site-packages\uvicorn\middleware\proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\applications.py", line 112, in __call__
    await self.middleware_stack(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\middleware\errors.py", line 184, in __call__
    raise exc
  File "c:\otree\ve-nodj\lib\site-packages\starlette\middleware\errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\exceptions.py", line 84, in __call__
    raise exc
  File "c:\otree\ve-nodj\lib\site-packages\starlette\exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\routing.py", line 582, in __call__
    await route.handle(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\routing.py", line 243, in handle
    await self.app(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\endpoints.py", line 30, in dispatch
    response = await handler(request)
  File ".\star_bug.py", line 15, in get
    raise TemplateRenderingError from e
star_bug.TemplateRenderingError

What I get is only the final exception:

ERROR: Exception in ASGI application
Traceback (most recent call last):
  File "c:\otree\ve-nodj\lib\site-packages\uvicorn\protocols\http\h11_impl.py", line 394, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "c:\otree\ve-nodj\lib\site-packages\uvicorn\middleware\proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\applications.py", line 112, in __call__
    await self.middleware_stack(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\middleware\errors.py", line 184, in __call__
    raise exc
  File "c:\otree\ve-nodj\lib\site-packages\starlette\middleware\errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\exceptions.py", line 83, in __call__
    raise exc from None
  File "c:\otree\ve-nodj\lib\site-packages\starlette\exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\routing.py", line 582, in __call__
    await route.handle(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\routing.py", line 243, in handle
    await self.app(scope, receive, send)
  File "c:\otree\ve-nodj\lib\site-packages\starlette\endpoints.py", line 30, in dispatch
    response = await handler(request)
  File ".\star_bug.py", line 15, in get
    raise TemplateRenderingError from e

Starlette uses raise exc from None in 4 places:

  • ExceptionMiddleware.__call__
  • ServerErrorMiddleware.__call__
  • WebSocketEndpoint.dispatch
  • _ASGIAdapter.send

It seems to me that in all 4 cases the from None could hide intentional context about users' errors.

Having the chained traceback would be very valuable both for reading the console output and for programmatic use
(I want to make a middleware that looks at the .__cause__ attribute of certain exceptions to deliver a more relevant error message). I'm happy to provide more details on my use case.

@oTree-org
Copy link
Contributor Author

Here is some more context my use case. Let's say I have a template containing this:

Your result is: {{ user.boom() }}

Now, let's say that user.boom() raises an error somewhere in its stack. I need to raise a TemplateRenderError, which contains the template debugging info (e.g. result.html, line 55). But I also need to show the original error that occurred inside user.boom(), e.g. a ZeroDivisionError. This is why I use raise TemplateRenderError(filename, lineno) from exc.

@oTree-org oTree-org changed the title Problems using chained exceptions "raise exc from None" interferes with exception chaining Jan 21, 2021
@Zamion101
Copy link

When using starlette.WebSocketEndpoint there is an while loop in dispatch that awaits for message = await websocket.receive() and when client-side closes connection abnormally it throws RuntimeError: Cannot call "receive" once a disconnect message has been received. which is raised from raise exc from None. It's not collected by internal exception handler.

@Zamion101
Copy link

Zamion101 commented Feb 20, 2021

Proposed changes to starlette.WebSocketEndpoint

WebSocketEndpoint
...
async def dispatch(self) -> None:
    websocket = WebSocket(self.scope, receive=self.receive, send=self.send)
    await self.on_connect(websocket)

    close_code = status.WS_1000_NORMAL_CLOSURE

    try:
        while True:
            message = await websocket.receive()
            if message["type"] == "websocket.receive":
                data = await self.decode(websocket, message)
                await self.on_receive(websocket, data)
            elif message["type"] == "websocket.disconnect":
                close_code = int(message.get("code", status.WS_1000_NORMAL_CLOSURE))
                break
    except Exception as exc:
        close_code = status.WS_1011_INTERNAL_ERROR
		# raise exc from None
		# call self.on_exception(...) rather than raising Exception immediately
        await self.on_exception(websocket, exc)
    finally:
        await self.on_disconnect(websocket, close_code)
async def decode(self, websocket: WebSocket, message: Message) -> typing.Any:

    if self.encoding == "text":
        if "text" not in message:
            await websocket.close(code=status.WS_1003_UNSUPPORTED_DATA)
			# raise RuntimeError("Expected text websocket messages, but got bytes")
			# call self.on_exception(...) rather than raising Exception immediately
            await self.on_exception(websocket, RuntimeError("Expected text websocket messages, but got bytes"))
        return message["text"]

    elif self.encoding == "bytes":
        if "bytes" not in message:
            await websocket.close(code=status.WS_1003_UNSUPPORTED_DATA)
			# raise RuntimeError("Expected bytes websocket messages, but got text")
			# call self.on_exception(...) rather than raising Exception immediately
            await self.on_exception(websocket, RuntimeError("Expected bytes websocket messages, but got text"))
        return message["bytes"]

    elif self.encoding == "json":
        if message.get("text") is not None:
            text = message["text"]
        else:
            text = message["bytes"].decode("utf-8")

        try:
            return json.loads(text)
        except json.decoder.JSONDecodeError:
            await websocket.close(code=status.WS_1003_UNSUPPORTED_DATA)
			# raise RuntimeError("Malformed JSON data received.")
			# call self.on_exception(...) rather than raising Exception immediately
            await self.on_exception(websocket, RuntimeError("Malformed JSON data received."))

    assert (
        self.encoding is None
    ), f"Unsupported 'encoding' attribute {self.encoding}"
    return message["text"] if message.get("text") else message["bytes"]

...
# New overridable method to handle exceptions internally/by user
async def on_exception(self, websocket: WebSocket, exception: Exception) -> None:
	"""Override to handle an error in websocket connection"""
	raise exception
my_app.py (Additional Method)
import logging
from starlette.endpoints import WebSocketEndpoint

logger = logging.getLogger("my_app")

class MessagesEndpoint(WebSocketEndpoint):
    
    async def on_connect(self, websocket):
        await websocket.accept()

    async def on_receive(self, websocket, data):
        pass
    
    async def on_disconnect(self, websocket, close_code):
        pass
    
    # Override `on_exception` method to handling Exception properly
    async def on_exception(self, websocket, exception):
		logger.error("Exception caught")

on_exception will override the actual on_exception method in WebSocketEndpoint and will be called when exception is raised.

@oTree-org
Copy link
Contributor Author

oTree-org commented Mar 17, 2021

Another fix for the issue would be to remove the from None in the following 4 places:

- ExceptionMiddleware.__call__
- ServerErrorMiddleware.__call__
- WebSocketEndpoint.dispatch
- ASGIAdapter.send

@tomchristie
Copy link
Member

It's possible that there might be a good reason for one or more of those cases to be a raise exc from None, but in general I'd agree with you, yup. It's probably worth a pull request, switching the behaviour of those. We can then review each case in that pull request, and check if any of them are "raise from None" for a good reason.

imjoehaines added a commit to bugsnag/bugsnag-python that referenced this issue Feb 15, 2022
This required bumping the Starlette version because of this issue:
encode/starlette#1114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants