graphql-python / graphql-server

This is the core package for using GraphQL in a custom server easily
MIT License
120 stars 72 forks source link

I'd like to change how errors are handled #10

Closed miracle2k closed 6 years ago

miracle2k commented 6 years ago

I strongly dislike the fact that this library seems to catch any and all exceptions, and returns them as an error response.

If I have a bug in my resolver code, this should be a 500 error, be tracked to Sentry, and the error message should not be exposed to the user.

If the user makes a mistake in their query, this should be a 400 error, with a clear message, and should not go to Sentry.

As it stands, this library (and thus all the integrations) do not seem to let me do this. I just get an error response, and I'd have to reverse engineer which class of error it is.

miracle2k commented 6 years ago

It seems what is really happening is this:

I guess what I want to do is override this format_error callback, and raise an exception there.

class MyGraphQLView(GraphQLView):

    @staticmethod
    def format_error(error):
        # graphene likes to wrap all of the exceptions, unwrap
        if isinstance(error, GraphQLLocatedError):
            error = error.original_error

        # Is this an error we want to pass through?
        if isinstance(error, GraphQLError):
            return graphql.error.format_error(error)
        else:
            # Unknown error, crash
            raise error
miracle2k commented 6 years ago

I think this is a bit confusing to achieve what I would perceive to be the "default" approach, but I am not sure if there is something actionable here right now from my point of view.

kaos commented 3 years ago

Most Excellent. This ought to be in the docs.

However, that seems to apply to the now legacy version https://github.com/graphql-python/graphql-core-legacy as I don't find the GraphQLLocatedError in the new core, which seems to loose the original exception type, as they only use the str(e.message) part of it!?

Ah, they've double wrapped it, in some cases. https://github.com/graphql-python/graphql-core/issues/106

EDIT:

Here's my take on it for graphql-core v3:

from graphql_server.flask import GraphQLView
from graphql import GraphQLError

class MyGraphQLView (GraphQLView):
    '''Credit: [@miracle2k Michael Elsdörfer](https://github.com/miracle2k)
    [graphql-python/graphql-server#10](https://github.com/graphql-python/graphql-server/issues/10#issuecomment-382809784)

    Adapted by @kaos
    '''
    @staticmethod
    def format_error(error):
        root_cause = error
        while isinstance(root_cause, GraphQLError) and root_cause.original_error:
            # work-around for https://github.com/graphql-python/graphql-core/issues/106
            root_cause = root_cause.original_error

        if isinstance(root_cause, GraphQLError):
            return GraphQLView.format_error(error)

        # treat non graphql errors as internal server errors
        raise error from root_cause