responder icon indicating copy to clipboard operation
responder copied to clipboard

TypeError in function routes get swallowed in _execute_route

Open steinnes opened this issue 6 years ago • 2 comments

If I implement a function view for a route which raises a TypeError for some reason, it seems to be caught by line 328 of responder/api.py (inside _execute_route), and the server returns 200 OK, with a json null body.

@api.route('/raise_me_up')
def raiser1(req, resp):
    raise TypeError
$ curl -v http://localhost:5001/raise_me_up
*   Trying ::1...
* TCP_NODELAY set
* Connection failed
* connect to ::1 port 5001 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 5001 (#0)
> GET /raise_me_up HTTP/1.1
> Host: localhost:5001
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 200 OK
< server: uvicorn
< date: Mon, 05 Nov 2018 11:10:13 GMT
< content-type: application/json
< transfer-encoding: chunked
<
* Connection #0 to host localhost left intact

Meanwhile the server log looks like this: INFO: ('127.0.0.1', 65440) - "GET /raise_me_up HTTP/1.1" 200

If however another type of exception is raised, the result is a 500 error:

@api.route('/raise_something')
def raiser2(req, resp):
    raise Exception("This is just another exception")
$ curl -v http://localhost:5001/raise_something
*   Trying ::1...
* TCP_NODELAY set
* Connection failed
* connect to ::1 port 5001 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 5001 (#0)
> GET /raise_something HTTP/1.1
> Host: localhost:5001
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 500 Internal Server Error
< server: uvicorn
< date: Mon, 05 Nov 2018 11:11:31 GMT
< content-length: 21
< content-type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact

And the server log contains the stack trace:

INFO: ('127.0.0.1', 65482) - "GET /raise_something HTTP/1.1" 500
ERROR: Exception in ASGI application
Traceback (most recent call last):
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/uvicorn/protocols/http/httptools_impl.py", line 387, in run_asgi
    result = await asgi(self.receive, self.send)
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/uvicorn/middleware/debug.py", line 80, in __call__
    await asgi(receive, self.send)
  File "/Users/ses/w/responder-test/rpt/lib/middleware/sqla.py", line 9, in __call__
    await self.inner(receive, send)
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/starlette/exceptions.py", line 69, in app
    raise exc from None
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/starlette/exceptions.py", line 61, in app
    await instance(receive, sender)
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/responder/api.py", line 227, in asgi
    req, scope=scope, send=send, receive=receive
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/responder/api.py", line 306, in _dispatch_request
    await self._execute_route(route=route, req=req, resp=resp, **options)
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/responder/api.py", line 328, in _execute_route
    await r
  File "/Users/ses/w/responder-test/venv/lib/python3.6/site-packages/responder/background.py", line 45, in __call__
    return await loop.run_in_executor(None, fn)
  File "/Users/ses/.pyenv/versions/3.6.6/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/Users/ses/w/responder-test/server.py", line 48, in raiser2
    raise Exception("This is just another exception")
Exception: This is just another exception

I'd like to fix this myself (and also dig into why I'm not seeing the stacktrace in my curl output, since the ExceptionMiddleware is hooked up since #157) but I'm not sure why a TypeError would be raised in this block: https://github.com/kennethreitz/responder/blob/master/responder/api.py#L321-L328

My first thought was that this happens if the view isn't a function, in which case I wonder why the route.is_function in line 319 evaluates to true. I'd like to understand more before I attempt to fix this (probably by not relying on exceptions for control flow).

steinnes avatar Nov 05 '18 11:11 steinnes

This is by design, currently.

kennethreitz avatar Nov 06 '18 10:11 kennethreitz

Since I seem to make a lot of mistakes resulting in TypeError when I'm developing, this afternoon I wrote a little hack/workaround to trap&wrap view originated TypeErrors. I'm sharing it here in case anyone else could benefit, and on the off chance @kennethreitz or someone here has some good feedback:

# errortmp.py
import traceback


class RouteTypeError(Exception):
    pass


class TypeErrorWrapper:
    def __init__(self, fn):
        self._real_fn = fn

    def _error_is_from_view(self, stack_entry):
        if stack_entry.name == "__call__" and stack_entry.filename == __file__:
            return False
        return True

    def __call__(self, *args, **kwargs):
        try:
            return self._real_fn(*args, **kwargs)
        except TypeError as e:
            summary = traceback.extract_tb(e.__traceback__)
            if self._error_is_from_view(summary[-1]):
                raise RouteTypeError(e).with_traceback(e.__traceback__)
            else:
                raise e

    def __getattribute__(self, attr):
        if attr in ['_real_fn', '_error_is_from_view']:
            return object.__getattribute__(self, attr)
        return getattr(object.__getattribute__(self, '_real_fn'), attr)


def route_patcher(api):
    original = api.route

    def route(*args, **kwargs):
        decorator = original(*args, **kwargs)

        def inner(f):
            return decorator(TypeErrorWrapper(f))
        return inner

    api.route = route

The way I use this:

# api.py

import responder
from myapp.lib.errortmp import route_patcher

api = responder.API(title="my app api", vversion="1.0", secret_key=os.environ['SECRET_KEY'])
route_patcher(api)

In my routes I import the api object from api.py, and the patcher has replaced the route decorator function with the one which injects theTypeErrorWrapper around the view function/endpoint.

steinnes avatar Nov 16 '18 15:11 steinnes