graphene: Error handling

Bump on this issue.

Error handling is a big deal when implementing any API and graphene’s middleware doesn’t offer any obvious way to do this.

A very common use case is catching all exceptions and stripping any internal error handling. This is crucial for security purposes: it’s not uncommon for stringified exceptions to have PII and other sensitive information.

The only way to do this in graphene is to decorate every resolver with error handling functionality or using the metaclass approach described above.

Ariadne, by contrast, treats this as a first class use case:

https://ariadnegraphql.org/docs/error-messaging

Is proper error handling somewhere on the roadmap?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

I gave up on graphql python, error handling is a basic feature

no

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

I know error handling is a bit complicated in graphql in general right now. For the purposes of this discussion, I think it’s useful to differentiate between expected error conditions and unexpected error conditions.

While it might make sense to swallow and format expected error conditions, not differentiating between something like an explicitly raised GraphQLError and an unexpected Exception/ValueError/KeyError/etc is unfavorable for a few reasons:

• As @clin88 stated, not leaking raw exceptions messages is security critical.
• Even if you’re okay with API users seeing raw exception strings, this makes it really hard to proactively detect and catch issues. Tools like Rollbar/Sentry/etc allow us to get proactive alerts about issues, and automatically collect a whole lot of relevant data for us to make debugging easier. This sort of all goes out the window unless you hack it into the middle and try to differentiate between explicitly raised GraphQLError’s and GraphQLError’s that were coerced from a different underlying exception.
• During development, pytest --pdb is a fantastic tool for understanding and debugging issues. When unexpected exceptions are swallowed, pdb becomes significantly less useful. While you still have a reference to the original_error buried in the execution result error list, there’s a lot of friction in actually getting at any useful data from it. At a minimum you have to:

  from traceback import print_tb
  print_tb(result.errors[0].original_error.__traceback__)
  

  After the 50th time typing variations of that line it gets a little old. But even with this boilerplate, you don’t get a nice pdb shell with frame navigation, variable inspection, etc.

One common approach for error handling in graphql is to make expected errors part of the schema design. Even doing that, though, there’s a handful of “expected” errors which are (correctly) raised by graphene/graphql-core and don’t neatly fall into that bucket.

While not as verbose, a very similar issue was sort of already raised in on graphql-core. The proposed resolution was optionally raising exceptions out of the library resolvers, but original_error was implemented instead. Given the constraint of graphql-core needing to closely align with graphql-js, I understand it might never make sense to make that change there. Instead, I do think it might be worth making that change here.

One way graphql-core deviates from graphql-js is that it allows for a custom execution context. From what I can tell, the default execution context is what’s responsible for swallowing exceptions. graphene’s Schema.execute calls graphql_sync, which accepts the optional execution_context_class kwarg. graphene could extend the default execution context to remove the exception swallowing code. This new execution context could either be used as the default execution_context_class (with the option of manually specifying the graphql-core error-swallowing context as a kwarg), or it could even just be provided as an optional class which can easily be added to Schema.execute calls and referenced in docs.

With exceptions not being swallowed at the lower level, it becomes much more obvious how to catch and handle them at the higher levels (which expose views), etc. For example, stripping internal errors can then be done the same way it’s done for other views served by whatever web framework exposes the graphql api.

The way all these libraries interact is a bit complicated so it’s hard to tell if I’m on the right track here. If this does seem like the right approach I’d be happy to file a PR with this change.

@jkimbo What do you think?

Indeed coming from graphql-js it’s hard to believe there is no proper solution to customize error handling

This is possible! You need to wrap the resolver like such

def wrap_resolver(cls, resolver):

    def wrapped_resolver(root, info, **args):
        try:
            with transaction.atomic():
                return resolver(root, info, **args)

        except Exception as e:
            return cls(errors=str(e))

    return wrapped_resolver


class CustomMutation(Mutation):

    class Meta:
        abstract = True


    @classmethod
    def __init_subclass_with_meta__(cls, resolver=None, **options):

        if not resolver:
            mutate = getattr(cls, 'mutate', None)
            assert mutate, 'All mutations must define a mutate method in it'
            resolver = get_unbound_function(mutate)
            resolver = wrap_resolver(cls, resolver)

        super(CustomMutation, cls).__init_subclass_with_meta__(resolver, **options)

        cls._meta.fields['errors'] = (Field(String, name='errors'))

Then you use CustomMutation as class that all your Mutations will subclass. This wraps the resolver and catches any exceptions. In my own implementation I have a custom Exception class that i’m looking for. But thats up for you to decide

It is actually possible to catch and handle resolver errors without needing to decorate or use a custom sub class for each every query / mutation. You do still need to use a custom meta class similar to others suggestions, but it only needs to be used when you build your schema. E.g.

def catch_errors(func):
    @functools.wraps(func)
    def wrapper(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except Error as e:
            # Handle errors here.

    return wrapper


class SchemaObjectType(ObjectType):

    class Meta:
        abstract = True

    @classmethod
    def __init_subclass_with_meta__(
            cls,
            interfaces=(),
            possible_types=(),
            default_resolver=None,
            _meta=None,
            **options):

        super().__init_subclass_with_meta__(
            interfaces, possible_types, default_resolver, _meta, **options)

        for f in cls._meta.fields:
            field = getattr(cls, f)
            if hasattr(field, "resolver"):
                field.resolver = catch_errors(field.resolver)

Then use this class to build your query and mutation objects. E.g.

class Query(SchemaObjectType):
    example = example_query_field()


class Mutation(SchemaObjectType):
    example_mutation = ExampleMutation.Field()


schema = graphene.Schema(query=Query, mutation=Mutation)

This clearly isn’t an ideal solution, but it does obviate the need to replace every ObjectType and Mutation individually.

Sorry for the delay. #1255 has been released in v3.0.0b6

Still an issue

@clin88 I agree that error handling is not handled (pun not intended!) well at the moment in graphene. There are no plans as far as I’m aware to improve it though because it feels like something that could devolve into configuration soup trying to cover everyone’s particular use case.

I am very open to hearing suggestions on how it could work though. Do you have any ideas?

@dspacejs You should be able to setup your python logging config to silence the errors in testing and development if you wish.

Does anyone have a workaround for this in the meantime? All exceptions raised are printed to the console during testing/development.

Some packages such as django-graphql-jwt raise exceptions for things like user authentication. Those kinds of exceptions are just for the API user to see, so it’d be nice to be able to raise exceptions without them polluting the console output.

@BossGrand yes, for mutations it works, and I solved it in the same way, but for queries doesn’t work. That’s the main problem, having to check errors differently for mutations and queries is not a good user experience.

So to keep them consistent we need to have a errors field for queries as well, if the root response cannot be extended, then subclassing ObjectType and adding an errors field, and use the custom ObjectType as the base type for every query result.

Queries may fail, but also, the resolvers of each field from the query response may fail, so you need a way of catching any of those errors, and some how populate the error field with them. Problem is you don’t control when fields are resolved, thus cannot force the error field to be resolved at the very end to make sure all the other resolvers were executed and catched (if they errored).

Its a bit confusing, but hopefully you got the idea.

Ive been looking at this issue for my current predicament. Would it make sense that instead of dropping the catchall Exception handler, like UnforgivingExecutionContext does, and allowing for passing in a custom exception handler that would allow a developer to at least intercept the exception for logging purposes, or just breakpointing in their own code?

I posted this in another thread but you guys might be interestead as well, it’s a small validation library on top of Graphene that follows Django Rest Framework’s structure (it does not require Django of course 😃 ): https://github.com/chpmrc/graphene-validator. Probably not ready for production use but I think it only needs some polishing. It does exactly what some of you have been trying to do (field level validation, even with nested inputs).

Let me know what you think!

So, just for the record, @bclermont I went with the decorator approach, catching every possible error on both queries and mutations (using a base class for both that wraps every resolver). In the error handler decorator I just catch, format and re-raise errors and finally in the global error handler I decide what data to output in the errors payload, depending on the type. I gave up with the errors field approach as is not possible to use it for queries.