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

Add Ariadne GraphQL error integration #2387

Merged
merged 22 commits into from
Oct 2, 2023
Merged

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Sep 21, 2023

Capture GraphQL errors when using Ariadne server side and add more context to them (request, response).

Example event in Sentry: https://sentry-sdks.sentry.io/issues/4510338932/events/220aadec5c604710b0041a14864e45b6/

One of three integrations for #2257, split from #2381

@sentrivana sentrivana marked this pull request as ready for review September 21, 2023 14:11
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicking, otherwise: really nice clean integration!

sentry_sdk/integrations/ariadne.py Outdated Show resolved Hide resolved
sentry_sdk/integrations/ariadne.py Outdated Show resolved Hide resolved
sentry_sdk/integrations/ariadne.py Outdated Show resolved Hide resolved
sentry_sdk/integrations/ariadne.py Show resolved Hide resolved
@sentrivana
Copy link
Contributor Author

sentrivana commented Sep 28, 2023

@antonpirker Made two additional changes since your review:

  • added an ignore_logger to prevent the logging integration from capturing the error first: 25495d5 and removed it from the tests e197c39
  • changed attaching request.data back so that the ariadne integration rewrites whatever is set there by the web framework integrations: 2ceb9b1 As I was testing stuff for docs I noticed that the web framework integrations sometimes seem to attach a wrong request body. Not sure exactly why. Making the ariadne integration overwrite it ensures that it's the correct one.

@sentrivana sentrivana enabled auto-merge (squash) October 2, 2023 13:13
@sentrivana sentrivana merged commit 7c74ed3 into master Oct 2, 2023
274 checks passed
@sentrivana sentrivana deleted the ivana/ariadne-integration branch October 2, 2023 13:23
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

Successfully merging this pull request may close these issues.

None yet

2 participants