Docs should not suggest returning graphql errors on authentication failure

General
,
1

I was implementing OAuth refresh token exchange in my app and came across this misguided example in the Apollo docs:

onError(({ graphQLErrors, networkError, operation, forward }) => {
  if (graphQLErrors) {
    for (let err of graphQLErrors) {
      switch (err.extensions.code) {
        // Apollo Server sets code to UNAUTHENTICATED
        // when an AuthenticationError is thrown in a resolver
        case "UNAUTHENTICATED":
          // Modify the operation context with a new token
          const oldHeaders = operation.getContext().headers;
          operation.setContext({
            headers: {
              ...oldHeaders,
              authorization: getNewToken(),
            },
          });
          // Retry the request, returning the new observable
          return forward(operation);
      }
    }
  }

  // To retry on network errors, we recommend the RetryLink
  // instead of the onError link. This just logs the error.
  if (networkError) {
    console.log(`[Network error]: ${networkError}`);
  }
});

As far as I understand from the GraphQL over HTTP draft spec, the server should be responding with 401 if it requires authentication, e.g. because a token expired, which must be handled as a network error. It would be wrong to return graphql field errors when authentication is required; resolvers should only throw authorization errors (meaning user is authenticated but not authorized to query a certain field or perform a certain mutation, while others may be allowed).

This example led me astray and I wasted time trying to handle token refresh in onError. The proper way appears to be to use a RetryLink with an async retryIf function that exchanges a refresh token on 401, then resolves to true. I can’t find any examples of async retryIf usage but I see in the code that it works and that’s what I had to do to get my refresh token exchange working without any errors popping up in the UI.

2

Hi @jedwards1211 :wave: thanks for the feedback! I’m glad you’re unblocked and it’s unfortunate that you had a bit of a side quest while implementing. Since this is fresh in your mind would you be interested in submitting a change via a PR? The maintainers would be eager to review it. Thanks a bunch :pray:

3

Yeah that’s a good idea, I’ll try to work on it tomorrow night!

2 Likes