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

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

Open
rscarrera27 opened this issue Jul 21, 2018 · 1 comment

Comments

@rscarrera27
Copy link

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
Copy link

zmwangx commented Apr 28, 2019

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.

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

No branches or pull requests

2 participants