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

How to capture exceptions? #124

Closed
MrTin opened this issue Mar 2, 2017 · 20 comments
Closed

How to capture exceptions? #124

MrTin opened this issue Mar 2, 2017 · 20 comments

Comments

@MrTin
Copy link

MrTin commented Mar 2, 2017

We're trying to forward exceptions to Sentry but it seems the graphene-django seems to be swallowing everything. Is there any way to catch the exception and send it to Sentry?

Thank you!

Regards,
Martin

@syrusakbary
Copy link
Member

syrusakbary commented Mar 5, 2017

Hi @MrTin,

There are few ways for catching the exceptions that GraphQL is swallowing (intentionally), the way I recommend is via Middleware:

class SentryMiddleware(object):
    def on_error(self, error):
        # report error to sentry
    def resolve(self, next, root, args, context, info):
       return next(root, args, context, info).catch(self.on_error)

Here are the docs for Middleware in graphene: http://docs.graphene-python.org/en/latest/execution/middleware/

Hope this helps!

@MrTin
Copy link
Author

MrTin commented Mar 29, 2017

Hi @syrusakbary

Sorry for my late reply – thank you very much for taking the time to reply.

If I understand this solution correct it requires me to add this middleware manually to each query? Or is there a way to set middlewares for all my queries in one place?

@sdcooke
Copy link

sdcooke commented Jul 5, 2017

@syrusakbary I gave this a go but catch was never actually called (I was just raising an exception in a resolve_ method) - it looked like the exceptions were just collected on the execution context. An alternative solution I found was to check execution_result.errors before returning the response which works for getting a list of exceptions (error.original_error), but doesn't work particularly well for Sentry because Sentry wants to use sys.exc_info(). So I have two questions:

  1. Do you think it would be possible to get sys.exc_info() into the errors returned on the execution result?
  2. Under what scenario do you think this middleware with catch would work?

Thanks!

@danielholmes
Copy link

I'm seeing the same issue as @sdcooke . Looking at this line: https://github.com/graphql-python/graphql-core/blob/master/graphql/execution/middleware.py#L57

def make_it_promise(next, *a, **b):
  return Promise.resolve(next(*a, **b))

It runs the execution, then places the result in a promise. So if next(*a, **b) returns a successful result it's wrapped in a promise but if there's an exception it bubbles instead.

Changing it to the following allows the middleware to catch errors:

def make_it_promise(next, *a, **b):
  return Promise.promisify(lambda: next(*a, **b))()

However that breaks down execution in other places.

i.e. Some parts of the code and @syrusakbary above seem to assume make_it_promise converts exceptions to rejected promises as in my version, but most of the code seems to assume the current behaviour of raising exceptions but wrapping success values.

@gamugamu
Copy link

I'm really struggling in catching exception on graphQL. The middelWare seems unhelpful as he is not triggered when there is an exception and the propagation stop before the middleware. (So it swallows all the client / server communication EVEN with a middleware)
That look like an anti-design pattern to not be able to get catch simply error from graphene. I know this is more related to graphene instead of graphene-django. But is there true helpful debugging example for graphene? This one is not working for my case (debugging 400 code error request)

@MrTin
Copy link
Author

MrTin commented Feb 25, 2018

Unfortunately, I never got around trying this out and ended up with my original solution using a custom GraphQLView.

You can grab the full solution and read why in my article on medium.

Here's also the code if you just want the solution:

# views.py
from graphene_django.views import GraphQLView
from raven.contrib.django.raven_compat.models import client as sentry_client


class SentryGraphQLView(GraphQLView):
    def execute_graphql_request(self, *args, **kwargs):
        """Extract any exceptions and send them to Sentry"""
        result = super().execute_graphql_request(*args, **kwargs)
        if result.errors:
            for error in result.errors:
                try:
                    raise error.original_error
                except Exception:
                    sentry_client.captureException()
        return result


# urls.py
from .views import SentryGraphQLView


urlpatterns = [
    url(r'^graphql', SentryGraphQLView.as_view(schema=schema)),
]

@picturedots
Copy link

Has this been resolved in graphene? I am seeing graphene exceptions in Sentry and I don't recall implementing any workarounds (with graphene==2.0.1, graphene-django==2.0.0, raven==6.6.0). Or is this a side-effect of using graphene_django.debug.DjangoDebugMiddleware setting for GRAPHENE["MIDDLEWARE"] ?

@wilk
Copy link

wilk commented Jul 3, 2018

The @syrusakbary 's solution works well, actually, with the latest version of graphql-core: https://github.com/graphql-python/graphql-core/blob/master/graphql/execution/middleware.py#L74
As you can see, the middleware execution is wrapped inside a well-constructed promise.

As a work-around, for previous versions of graphql-core, try this:

import sys
import logging

class SentryMiddleware(object):
  def resolve(self, next, root, info, **args):
    try:
      return next(root, info, **args)
    except:
      err = sys.exc_info()
      logging.error(err)
      return err[1]

@zbyte64
Copy link
Collaborator

zbyte64 commented Sep 12, 2018

Is it a good idea to add logging statements to views.py so that this can be done through configuring django LOGGING?

@shabbyrobe
Copy link

The middleware solution does not catch exceptions in dataloaders; next returns a promise.

@vojto
Copy link

vojto commented Feb 4, 2019

Here's @MrTin's solution updated to Sentry unified Python SDK:

from sentry_sdk import capture_exception

class SentryGraphQLView(GraphQLView):
    def execute_graphql_request(self, *args, **kwargs):
        """Extract any exceptions and send them to Sentry"""
        result = super().execute_graphql_request(*args, **kwargs)
        if result.errors:
            for error in result.errors:
                try:
                    raise error.original_error
                except Exception as e:
                    capture_exception(e)
        return result

# In routes:

path('graphql', csrf_exempt(SentryGraphQLView.as_view(graphiql=True))),

@picturedots
Copy link

After I updated to sentry_sdk, I started receiving the correct data exceptions in Sentry, without any addition changes to views or middleware. The errors were double-reported as GraphQLLocatedError errors, so I added a filter to remove them:

from graphql.error import GraphQLLocatedError

def before_send(event, hint):
    """
    Filters GraphQLLocatedError events to prevent double reporting at Sentry.
    """
    if 'exc_info' in hint:
        _exc_type, exc_value, _tb = hint['exc_info']
        if isinstance(exc_value, GraphQLLocatedError):
            return None
    return event

sentry_sdk.init(before_send=before, ...)

@mvanlonden
Copy link
Member

Closing since this issue is not specific to graphene-django

@sandwichsudo
Copy link
Contributor

sandwichsudo commented Jul 2, 2019

In case this helps someone - the class based middleware didn't work for me, but this function based middleware did:

def log_error(error):
    logger.error(error) # we've set this up to send issues to sentry
    raise graphql.GraphQLError("An internal error occurred")


def uncaught_exception_middleware(next, root, info, **args):
    return next(root, info, **args).catch(log_error)

then in our settings:

GRAPHENE = {
    "MIDDLEWARE": [
        "octoenergy.interfaces.apisite.graphql.middleware.uncaught_exception_middleware",
    ],
}

@tony
Copy link

tony commented Feb 14, 2020

Hi there, chiming in - if by chance you're seeing stuff like this:

image

Check around for logger and especially graphql.execution.utils. ExecutionContext.report_error

Your sentry may be set to listen to logging.Logger.error.

https://github.com/graphql-python/graphql-core/blob/v2.3.1/graphql/execution/utils.py#L155

To get around this, it may be necessary to ignore this logger through sentry:

from sentry_sdk.integrations.logging import ignore_logger

ignore_logger("graphql.execution.utils")

Credits go to @malthejorgensen if this is helpful to you

@honi
Copy link

honi commented May 8, 2020

Combining @tony's snippet with some previously posted snippets I'm using this config to properly log errors to sentry without changing anything in graphene. This config will log the errors only once in Sentry and they will have the full inspectable stacktrace.

# Ignore this logger because it logs errors as strings instead of actual errors
# and we lose a lot of Sentry's features. Instead use SentryMiddleware.
from sentry_sdk.integrations.logging import ignore_logger
ignore_logger('graphql.execution.utils')


class SentryMiddleware(object):
    """
    Properly capture errors during query execution and send them to Sentry.
    Then raise the error again and let Graphene handle it.
    """

    def on_error(self, error):
        capture_exception(error)
        raise error

    def resolve(self, next, root, info, **args):
       return next(root, info, **args).catch(self.on_error)


# In your settings.py:
GRAPHENE = {
    'MIDDLEWARE': [
        'core.schema.SentryMiddleware',
        # Other middleware if any.
    ],
    # Other graphene specific settings if any.
}

Versions:
django==3.0.5
graphene==2.1.8
graphene-django==2.9.1
sentry-sdk==0.14.3

@ulgens
Copy link
Collaborator

ulgens commented May 9, 2020

graphql-python/graphql-core-legacy#269

@loweryjk
Copy link

Sure would be nice if middleware.on_error was in the docs!

mmiermans added a commit to Pocket/recommendation-api that referenced this issue Feb 3, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
This doesn't work yet, but according to some Github issue it's the right direction:
graphql-python/graphene-django#124
mmiermans added a commit to Pocket/recommendation-api that referenced this issue Feb 3, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* [BACK-585] Add Sentry middleware to GraphQL

This doesn't work yet, but according to some Github issue it's the right direction:
graphql-python/graphene-django#124

* WIP local Sentry debugging

* Fixed logging exceptions to Sentry

* Ignore graphql logger

* Add comment

* Added comment about ignoring graphql logger
@danmoz
Copy link

danmoz commented May 25, 2023

Sure would be nice if middleware.on_error was in the docs!

I guess there was a reason this was undocumented -- this appears to have been removed in Graphene v3

@dicknetherlands
Copy link

dicknetherlands commented Sep 22, 2023

This workaround works for me - it's not Sentry but could be adapted:

class ExceptionLoggingGraphQLView(GraphQLView):
    def execute_graphql_request(self, *args, **kwargs):
        response = super().execute_graphql_request(*args, **kwargs)
        if response:
            for ex in response.errors or []:
                logging.error(ex, exc_info=ex)
        return response

urlpatterns = [
    path(
        "gql", ExceptionLoggingGraphQLView.as_view(schema=schema),
    ),
]

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