Skip to content

Latest commit

 

History

History
282 lines (208 loc) · 7.41 KB

0087-graphql-errors.md

File metadata and controls

282 lines (208 loc) · 7.41 KB

Summary

Add Request and Response bodies to GraphQL errors.

This feature is opt-in by default due to PII concerns.

Motivation

The Request interface contains information on a HTTP request related to the event.

The Request contains the data field which is the request body. Can be given as string or structural data of any format.

The Response interface contains information on a HTTP response related to the event.

However, the Response interface does not contain the data field which is the response body.

Background

This is necessary to implement the GraphQL Client Errors with syntax highlight.

The Request and Response bodies could contain PII.

Request body example (query language only):

query { 
  viewer { 
    login
  }
}

Response body example:

{
  "data": {
    "viewer": {
      "login": "foo"
    }
  }
}

A Request example with an error (query language only):

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

Response body example:

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

Using the locations and the request body (query language only), we can highlight the error in the request body.

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

Request body example, full body (not only the query language):

{
  "query": "{\n  viewer {\n    login\n  }\n}",
  "variables": {}
}

The Request body can also contain variables.

{
  "query": "{\n  viewer {\n    login\n  }\n}",
  "variables": {
    "login": "foo"
  }
}

Because of that, the Request and Response bodies should be sent to Sentry.

Supporting Data

See JIRA issues, not disclosing here because they could contain PII.

Related issues and discussions:

GraphQL Support

Support for GraphQL errors

Send request body for http client integrations and similar

Proposal (Option 1)

The proposal is adding a data field in the Response interface and adding an api_target field in the Request interface.

By doing this, we can copy the data scrubbing rules from the Request interface.

When the api_target is graphql, Relay will run data scrubbing in the Response#data field based on the Request#data[variables] field if the query is parameterized, otherwise Relay will run data scrubbing in the Response#data[data] field directly since the most important is Response#data[errors] anyways.

Request example:

{
  "request": {
    "method": "POST",
    "url": "http://absolute.uri/graphql",
    "data": {
      "foo": "bar"
    },
    "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;",
    "headers": {
      "content-type": "text/html"
    },
    "api_target": "graphql"
  }
}
  • api_target: Can be given as string. Values can be graphql, rest, etc.

Response example:

{
  "contexts": {
    "response": {
      "type": "response",
      "status_code": 500,
      "body_size": 1000,
      "data": {
        "foo": "bar"
      }
    }
  }
}
  • data: Can be given as string or structural data of any format.

The Response interface keeps arbitrary fields, it is backwards compatible with the current implementation.

Must have

The fields Request#data and Response#data could contain PII and they should run data scrubbing aggressively.

Session Replay already sends the request and response bodies, so we can use the same data scrubbing rules.

Since GraphQL is a well defined spec, we can also scrub the GraphQL fields.

Data scrubbing is going to be done on Relay to avoid the data scrubbing rules duplication on every SDK, it's also easier to roll out bug fixes on Relay.

Request example:

{
  "query": "{\n  viewer {\n    login\n  }\n}",
  "variables": {
    "login": "foo"
  }
}

Response example:

{
  "data": {
    "viewer": {
      "login": "[Filtered]"
    }
  }
}

In this case, we only need to use the Request variables and its keys to scrub the Response data.

Drawbacks

Envelopes (Events) contain way lower size limits. The data fields could be large and it could be a problem.

SDKs should discard large and binary bodies by default, using the maxRequestBodySize and maxResponseBodySize (it'll be added) options.

The difference is that for GraphQL errors, this should be enabled by default.

Appendix

Removed Proposals

Option 2

Add a new graphql interface to Contexts.

{
  "contexts": {
    "graphql": {
      "type": "graphql",
      "data": "...",
    }
  }
}

We'd need to duplicate or still use some fields from the Request and Response interface.

Size limits would still be a problem.

Option 3

Add a new envelope item for GraphQL.

{"event_id":"9ec79c33ec9942ab8353589fcb2e04dc"\n
{"type":"graphql","length":41,"content_type":"application/json"}\n
{"request":"foo","response":"bar"}

This would not be back compatible and must be added to all SDKs.

Option 4

Add Request and Response bodies as attachments.

{"event_id":"9ec79c33ec9942ab8353589fcb2e04dc"\n
{"type":"attachment","length":10,"content_type":"application/json","filename":"request.txt"}\n
foo
{"type":"attachment","length":10,"content_type":"application/json","filename":"response.txt"}\n
bar

Attachments have to be special cased in Sentry, seems hacky, we do that with screenshots already.

Unresolved questions

  • Do we need to send the GraphQL scheme to Sentry in order to do data scrubbing properly?
  • Should we send the Request and Response as different envelope items? (avoid the size limit)
  • Should PII be scrubbed in the SDK instead?