Skip to content
This repository has been archived by the owner on Oct 5, 2024. It is now read-only.

GraphQL errors: // we couldn't format it nicely #7

Closed
wesbillman opened this issue Feb 14, 2023 · 5 comments
Closed

GraphQL errors: // we couldn't format it nicely #7

wesbillman opened this issue Feb 14, 2023 · 5 comments
Labels
GraphQL Description for sentry_link

Comments

@wesbillman
Copy link

@ueman thanks for this! It's exactly what I needed for our Sentry setup.

I've noticed that my GraphQL errors are getting reported to Sentry as <unlabeled event>s
Screenshot 2023-02-14 at 11 55 24 AM

Digging in a bit it seems like errors.toSentryExceptions(request, response) is failing to return exceptions so we hit the // we couldn't format it nicely block.

I'm testing with a fairly simple error just to verify everything is setup correctly.

{
  "errors": [
    {
      "message": "Sentry test",
      "path": ["testPath"],
      "extensions": { "code": "INTERNAL_SERVER_ERROR" }
    }
  ],
  "data": { "testPath": null, "__typename": "Mutation" }
}

Here's a screenshot from the debugger. the exceptions will be empty here so we end up in the else block
Screenshot 2023-02-14 at 11 50 53 AM

@ueman
Copy link
Owner

ueman commented Feb 14, 2023

Thanks for the detailed report. It's always appreciated to have such reports.

What would you like to have as the error message in Sentry? Sentry message or INTERNAL_SERVER_ERROR? Would you like to contribute via PR?

@wesbillman
Copy link
Author

Yup! Happy to contribute a PR. I wanted to check with you on if there is something you were expecting with the current behavior before changing things up.

I think I lean towards having the error.message show up there, but it's a bit more tricky with an array of errors. In that case, I'm not sure what message would be best :)

@ueman
Copy link
Owner

ueman commented Feb 14, 2023

I wanted to check with you on if there is something you were expecting with the current behavior before changing things up.

So far it seems like a case I haven't yet run into.

but it's a bit more tricky with an array of errors

One Sentry event can have multiple exceptions, so multiple GraphQL errors are actually possible from the technical side. Every GraphQL error corresponds to one Sentry exception, and all of those exceptions are sent in one event.

If you want, you can also give input on the following issues in the Sentry repo which are about improved GraphQL support in Sentry:

@wesbillman
Copy link
Author

wesbillman commented Feb 15, 2023

@ueman I took a deeper look today at my code and as you might have already guessed this was caused by not having a locations field in our errors response. I feel like that is an error on our side so maybe there's no change required here.

We could think about what an exception might look like if it's missing the location information, but I think what you have now works well.

@ueman ueman added the GraphQL Description for sentry_link label Feb 26, 2023
@ueman
Copy link
Owner

ueman commented Jun 14, 2023

Since Sentry now properly supports GraphQL, I'll close this issue as it is no longer relevant because the way issues are reported will change.

@ueman ueman closed this as completed Jun 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
GraphQL Description for sentry_link
Projects
None yet
Development

No branches or pull requests

2 participants