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

TypeError in function routes get swallowed in _execute_route #194

Closed
steinnes opened this issue Nov 5, 2018 · 2 comments
Closed

TypeError in function routes get swallowed in _execute_route #194

steinnes opened this issue Nov 5, 2018 · 2 comments
Labels
bug Something isn't working

Comments

@steinnes
Copy link
Contributor

steinnes commented Nov 5, 2018

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).

@kennethreitz
Copy link
Owner

This is by design, currently.

@kennethreitz kennethreitz added the bug Something isn't working label Nov 6, 2018
@steinnes
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants