apollographql / apollographql/federation
Gateway: Error handling incompatible with Apollo Service
- Dominant language
- TypeScript
- Stars
- 727
- Forks
- 276
- Avg merge
- 1h 47m
- Merged PRs (30d)
- 1
Description
### Expected
Errors from a service should be transparently proxied to the client through the gateway.
### Actual
Errors from services are wrapped inside another error, their root cause hard to find, and the original http status code lost.
### Description
We are using `gateway` + `federation` to do schema stitching between two services. This works great, except when there are non-200 responses in one of the services. i.e., throwing an authentication error from context creation.
Given an error response from a service that looks like:
```json
{
"errors":[
{
"message":"Context creation failed: JsonWebTokenError: invalid token",
"extensions":{
"code":"UNAUTHENTICATED",
"exception": { }
}
}
]
}
```
`HTTP status code: 400`
Gateway transforms it to:
```json
{
"errors": [
{
"message": "400: Bad Request",
"extensions": {
"code": "INTERNAL_SERVER_ERROR",
"response": {
"url": "http://localhost:4000/",
"status": 400,
"statusText": "Bad Request",
"body": {
"errors": [
{
"message": "Context creation failed: JsonWebTokenError: invalid token",
"extensions": {
"code": "UNAUTHENTICATED",
"exception": { }
}
}
]
}
},
"exception": { }
}
}
],
"data": null
}
```
`HTTP Status Code: 200`
Three things happened:
1. HTTP status code changed.
2. Top Level `extensions.code` changed.
3. It is now significantly harder to realize that the root cause of the issue was unauthorized.
`apollo-server`, seemingly purposefully, does not set the HTTP status codes that one might expect when throwing an error: i.e., `AuthenticationError`. I say purposefully based on my read of: https://github.com/apollographql/apollo-server/issues/1709
This is okay, and I can see the rationale behind it, however this directly conflicts with how gateway checks for errors in remote graphql data sources: https://github.com/apollographql/apollo-server/blob/6c72193a7ace54b8522c69f3ba26ab38da37ff9d/packages/apollo-gateway/src/datasources/RemoteGraphQLDataSource.ts#L203-L227
The problems with this function are:
1. Gateway is explicitly looking at the http status code of the responses to see how it needs to proxy the errors, but the apollo server explicitly does not use those status codes that way.
2. Any errors from the service get wrapped instead of merely stitched together as a successful response would.
This makes error handling in the client significantly harder, as one can't as easily detect unauthorized errors to force the user to log out or refresh the token. This is also bad UX because the implementation detail of the gateway is now leaking through. Successful responses do not have such leakage of the implementation. The client shouldn't care if the it's calling a gateway or a service directly, but now we are forcing it to care due to error handling.
If we are doing something unidiomatic/surprising that is leading to this weird behavior based on the description given above then it would be great to have the idiomatic way documented as I couldn't find much information in the gateway docs.
Contributor guide
Research direction
Start with packages/apollo-gateway/src/datasources/RemoteGraphQLDataSource.ts at lines 203-227, then reproduce the gateway/federation response for a service returning a non-200 GraphQL error. Compare the service and gateway responses, and consider the issue complete when the gateway preserves the relevant error information and HTTP status behavior described here.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- graphql, typescript
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100