graphql-python / flask-graphql

Adds GraphQL support to your Flask application.
MIT License
1.32k stars 140 forks source link

Flask's app.errorhandler decorator dosen't work for GraphQLLocatedError #49

Open rscarrera27 opened 6 years ago

rscarrera27 commented 6 years ago

I am making an extension library flask-graphql-auth. and error handler by using flask's app.errorhandler is being planed for the 1.0 release. I checked that if my library caused an exception like JWTDecodeError, an GraphQLLocatedError occurred. However, the flask can not handle the error by app.errorhandler. Is this a bug?

        @app.errorhandler(GraphQLLocatedError)
        def handle(e):
            return 401

I wrote this code. but stacktrace appears :(

...
Traceback (most recent call last):
  File "C:\Users\Lewis\OneDrive\Documents\Development Repos\flask-graphql-auth\venv\lib\site-packages\graphql\execution\executor.py", line 330, in complete_value_catching_error
    exe_context, return_type, field_asts, info, result)
  File "C:\Users\Lewis\OneDrive\Documents\Development Repos\flask-graphql-auth\venv\lib\site-packages\graphql\execution\executor.py", line 383, in complete_value
    raise GraphQLLocatedError(field_asts, original_error=result)
graphql.error.located_error.GraphQLLocatedError: Signature verification failed
127.0.0.1 - - [21/Jul/2018 12:48:30] "POST /graphql? HTTP/1.1" 200 -
zmwangx commented 5 years ago

I just encountered this issue myself and looked into it. (My use case: I have a field with two mutually exclusive arguments, and need to return 400 when both are present. Isn't really possible on the schema level, and hence needs to be handled in a resolver.)

The problem apparently is deep down in graphql-core. See complete_value_catching_error (https://github.com/graphql-python/graphql-core/blob/d8e9d3abe7c209eb2f51cf001402783bfd480596/graphql/execution/executor.py#L461-L495). An exception raised in a resolver simply isn't treated the same as a schema level error (which causes the ExecutionResult to be invalid); the exception is logged and the executor moves on with a None. The exception ends up in ExecutionResult.errors too. But since it's not an uncaught exception, Flask's error handler cannot kick in.

I'm not sure if one can force an invalid down at the complete_value_catching_error level since I only read a little bit of the code and didn't try to follow the data flow, but that won't help with non-400 status codes anyway. Instead, the solution is higher up, in graphql_server.format_execution_results (https://github.com/graphql-python/graphql-server-core/blob/master/graphql_server/__init__.py#L144-L158):

def format_execution_result(
    execution_result,  # type: Optional[ExecutionResult]
    format_error,  # type: Optional[Callable[[Exception], Dict]]
):
    # type: (...) -> GraphQLResponse
    status_code = 200

    if execution_result:
        if execution_result.invalid:
            status_code = 400
        response = execution_result.to_dict(format_error=format_error)
    else:
        response = None

    return GraphQLResponse(response, status_code)

Note that you can monkeypatch this function to check for any custom exception, and even deny data in the response if you want to. Here's an example (say we want 400 on BadRequest and 401 on Unauthorized):

import graphql_server
from graphql_server import GraphQLResponse

class BadRequest(Exception):
    pass

class Unauthorized(Exception):
    pass

def format_execution_result(
    execution_result,  # type: Optional[ExecutionResult]
    format_error,  # type: Optional[Callable[[Exception], Dict]]
):
    # type: (...) -> GraphQLResponse
    status_code = 200

    if execution_result:
        if execution_result.invalid:
            status_code = 400
        for e in execution_result.errors:
            if isinstance(e, BadRequest):
                status_code = 400
            if isinstance(e, Unauthorized):
                status_code = 401
        # Deny data entirely on 4XX if you want to.
        if status_code != 200:
            execution_result.invalid = True
        response = execution_result.to_dict(format_error=format_error)
    else:
        response = None

    return GraphQLResponse(response, status_code)

# Monkeypatch
graphql_server.format_execution_result = format_execution_result

Personally I've decided to not bother with the status code after all... But maybe my investigation could help someone.