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

Catching exceptions #202

Open
leebenson opened this issue Aug 7, 2018 · 1 comment
Open

Catching exceptions #202

leebenson opened this issue Aug 7, 2018 · 1 comment

Comments

@leebenson
Copy link

leebenson commented Aug 7, 2018

Using Graphene with graphql-core 2.0, I was able to write middleware that would catch errors:

class ErrorHandlerMiddleware():
    def resolve(self, next, root, info, **args):
        try:
            return next(root, info, **args)
        except Exception as ex:
            
            # log error with Sentry
            info.context.sentry.captureException()

            # Return a generic rejection
            return GraphQLError("System error. Please try again later.")

It wasn't perfect. Both the original exception and GraphQLError would wind up appearing as Sentry issues, but it'd at least get reported and send back a generic error to the user.

I've since bumped graphql-core to 2.1 to take advantage of the logging improvements, but I'm not clear on how the above could be achieved in 2.1? With the latest version, the original exception winds up in the message field of the response... potentially exposing sensitive info, like SQL errors, etc.

Could you please post an example in the README of how one might write middleware or use the new logging set-up to capture errors and replace the response with a generic message? I think that would be a really common use-case that many would find useful.

@leebenson
Copy link
Author

leebenson commented Aug 16, 2018

FWIW, this is what I wound up doing. Can anyone see an issue with this approach?

def error_handler(ex):
    print("original exception", ex) # replace this with third-party logging
    raise Exception("System error") # some error message to show to users

class ErrorMiddleware:
    def resolve(next, root, info, **args):
        res = next(root, info, **args)

        return res.then(None, error_handler)

With the print line in the error handler being replaced with Sentry logging, and the new exception being raised to mask the original (hiding sensitive details like SQL queries, etc)

It still logs out graphql.error.located_error.GraphQLLocatedError: System error to stderr but setting the main logger to silence those should take care of it.

I'll leave this open to get consensus on this approach.

This was referenced May 21, 2019
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

1 participant