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

Support for GraphQL errors #33723

Closed
ueman opened this issue Apr 19, 2022 · 11 comments
Closed

Support for GraphQL errors #33723

ueman opened this issue Apr 19, 2022 · 11 comments

Comments

@ueman
Copy link

ueman commented Apr 19, 2022

Problem Statement

I would like to be able to report GraphQL errors and see them nicely formatted.

According to the GraphQL docs, it returns structured error data. See this and this document.

Example time by using GitHubs GraphQL explorer:

The following query returns a result:

# Query
query { 
  viewer { 
    login
  }
}
// result
{
  "data": {
    "viewer": {
      "login": "ueman"
    }
  }
}

If I now introduce an error to the query

# Query
query { 
  viewer { 
    # note the removed `n` at the end
    logi
  }
}

I'm greeted with this error

{
  "errors": [
    {
      "path": [
        "query",
        "viewer",
        "logi"
      ],
      "extensions": {
        "code": "undefinedField",
        "typeName": "User",
        "fieldName": "logi"
      },
      "locations": [
        {
          "line": 8,
          "column": 5
        }
      ],
      "message": "Field 'logi' doesn't exist on type 'User'"
    }
  ]
}

Solution Brainstorm

If I report the query and the error response, I would like to be able to see the error highlighted in the query.

@getsentry-release
Copy link

Routing to @getsentry/owners-ingest for triage. ⏲️

@tonyo
Copy link
Contributor

tonyo commented Apr 19, 2022

@ueman Do you perhaps want to have better support for GraphQL in a specific SDK?
As in, the error that you receive will be handled by a GraphQL library that you use in your code, and then it's up to the SDK to actually process it, format if necessary, and send to Sentry.

@getsentry-release
Copy link

Routing to @getsentry/team-web-sdk-backend for triage. ⏲️

@ueman
Copy link
Author

ueman commented Apr 19, 2022

@tonyo No, that's something I've already done myself. I've written code to report GraphQL errors here and it currently looks like the following screenshot:
Bildschirmfoto 2022-04-19 um 17 58 53

Based on the error from the response, I would like to have the following visualization in Sentry for the request object:

Something like this based on the path in the error
Artboard

Or based on the location from the error:
Artboard Copy

(The query can also be tested against GitHubs GraphQL explorer)

I hope that makes my feature request more understandable 😅

Also, if there are already ways to improve on this reporting, please let me know.
I haven't yet found a way to improve those reports because GraphQL errors doesn't match the stacktrace or template payloads.

@tonyo
Copy link
Contributor

tonyo commented Apr 19, 2022

Thanks for clarifying, will let frontend folks triage it, then!

@getsentry-release
Copy link

Routing to @getsentry/app-frontend for triage. ⏲️

@getsentry-release
Copy link

Routing to @getsentry/workflow for triage. ⏲️

@scttcper
Copy link
Member

@ueman thanks for the feature request, I've added a ticket to improve the graphql errors to the workflow team backlog

@marandaneto
Copy link
Contributor

marandaneto commented Apr 24, 2023

Related issue: getsentry/sentry-dart#1126

Discussion: #38913

Related notion page: https://www.notion.so/sentry/GraphQL-proposal-d6e5846f30434770903cf3af20bc2568

Related JIRA issues:
https://getsentry.atlassian.net/browse/MOBILE-322?jql=text+%7E+%22graphql%22
https://getsentry.atlassian.net/browse/FEEDBACK-1155?jql=text+%7E+%22graphql%22
https://getsentry.atlassian.net/browse/FEEDBACK-1324?jql=text+%7E+%22graphql%22
https://getsentry.atlassian.net/browse/FEEDBACK-1417?jql=text+%7E+%22graphql%22
https://getsentry.atlassian.net/browse/PERF-1805?jql=text+%7E+%22graphql%22
https://getsentry.atlassian.net/browse/WOR-1871?jql=text+%7E+%22graphql%22

Proposal:

  • Highlight the GraphQL payload (query) in the transaction or span description, and GraphQL errors using the location field (FE work)
    • Example GraphQL Support #38913 (comment)
    • For Transaction and span description, a language highlight (similar to markdown) would be better than a plain String, same for the HTTP client errors payload
    • If it requires structured data rather than just a String (Either SDK or Ingest work)
  • Parameterized GraphQL payload (Either SDK or Ingest work)
  • PII stripping GraphQL payload (Either SDK or Ingest work)
  • Request and Response payloads as a different envelope item (SDK and Ingest work)
    • Due to the event size limit (1MB right now).
    • The request should not be a problem, it's similar to a SQL query, but the response can be bigger
  • Request and Response payload in the Issue page
  • Detection of performance issues
    • See the notion page, there are a few ideas.

An RFC is needed, we can split up the work for HTTP Client errors and APM.
There are changes on the SDKs, FE and BE (ingestion).

@marandaneto
Copy link
Contributor

Relay PR getsentry/relay#2141

@marandaneto
Copy link
Contributor

Closed by:

BE changes getsentry/relay#2141
FE changes: #50764
Apollo3 Kotlin SDK changes, getsentry/sentry-java#2775 and getsentry/sentry-java#2781
Apollo3 Kotlin Docs: getsentry/sentry-docs#7135

Let us know what you think.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants