-
Notifications
You must be signed in to change notification settings - Fork 767
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
Raising exceptions without logging them #780
Comments
Accidentally closed this issue. |
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. |
Responding to remove the bot's label. I still have this issue and it's important. |
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. |
Commenting to remove the |
Hey @dspacejs , I removed the label. I also having problems with exact same issue for a long time. graphql-core package has other issues with error logging, i created a PR for that graphql-python/graphql-core-legacy#269 but it's for the legacy version of the package and i don't think it will be ever merged. I agree on the importance of the issue you mention, but i think it would be better to revisit this after graphene 3 update. |
@ulgens thanks. What will Graphene 3 update exactly that will address this? |
@dspacejs I don't think the update has anything to address this issue, but if the issue still exist after the update, fixes on the core package will be more likely to get merged. |
Definitely interested in a solution for this myself - don't necessarily need to raise errors instead of logging them, but figuring out a better way for sentry to at least track them (vs needing to look at logs) would be super helpful. |
The problem is that
|
Any updates on this issue? Running into the exact issue @dspacejs described |
Would also like to have a solution for this issue |
No idea what's going on here. I think this issue should be prioritised, it's been over 2 years. |
I'm currently using the following logging filter to filter out class GraphQLLogFilter(logging.Filter):
def filter(self, record):
if record.msg and 'GraphQLLocatedError' in record.msg:
return None
return True
# settings.py
LOGGING = {
'version': 1,
'disable_existing_loggers': False,
'handlers': {
'console': {
'level': 'DEBUG',
'class': 'logging.StreamHandler',
},
},
'filters': {
'graphql_log_filter': {
'()': GraphQLLogFilter,
}
},
'loggers': {
'graphql.execution.utils': {
'level': 'WARNING',
'handlers': ['console'],
'filters': ['graphql_log_filter'],
},
},
} Nevertheless this exceptions should not end up in logging at all. I hope for a solution. |
Has this issue been fixed in the current version? It would be nice to have more control over the verbosity of these errors. The stacktraces really worsen my logs readibility. |
Whenever exceptions are raised they're logged in the console (and in Sentry if it's used).
Many of these exceptions are only intended to be shown to the user. For example,
django-graphql-jwt
raises thePermissionDenied
exception for thelogin_required
decorator.The problem is this pollutes the console output during testing/development and logs unnecessary errors to Sentry. For exceptions such as the example above, that's only intended to be shown to the user.
As a workaround I've tried writing middleware that catches any exceptions thrown:
But if an exception is caught, it no longer populates the
errors
field in query/mutation output. Therefore all errors are logged, there's no way to conditionally log exceptions.This means the only solution is to create a logging filter like the following:
But this doesn't work either because
record.exc_info
isNone
whenever an error is thrown with Graphene, therefore it's not possible to conditionally filter out exceptions based on their type.Is there a solution for this? Seems like it'd be a common issue but I've had trouble finding one. Something like this would be nice to be able to implement. Thanks in advance.
The text was updated successfully, but these errors were encountered: