apollographql / apollo-link

:link: Interface for fetching and modifying control flow of GraphQL requests
https://www.apollographql.com/docs/link/
MIT License
1.44k stars 344 forks source link

Response field is undefined for networkError [apollo-link-error] #855

Open ViggoV opened 6 years ago

ViggoV commented 6 years ago

There seems to be a bug in either the onError function or its documentation. The issue has already been mentioned in #698 but I thought I would flesh it out a bit in hopes of a resolution.

When intercepting an error using onError from either apollo-boost or directly from apollo-link-error the documentation states that the error object contains a response object, which can be used to ignore errors.

I have marked the docs checkbox since this might be intended behaviour, but as the docs mentions the response object in a sub-paragraph of the networkError section it should probably be clarified a bit if that is the case.

Expected Behavior Setting response.errors = null; should prevent the error from propagating without throwing another error.

Actual Behavior When catching a networkErrorthe response field is undefined, causing syntax errors when trying to set response.errors. For reasons that are beyond me at the moment this actually does prevent the network error from propagating, but throws another error instead.

In my setup (MacOS 10.14.1 w. Chrome 70.0.3538.77) setting response.errors = null will throw a Uncaught TypeError: Cannot set property 'errors' of undefined and response = { errors: null } will throw a Uncaught Error: "response" is read-only.

skaermbillede 2018-11-06 kl 15 08 10

skaermbillede 2018-11-06 kl 15 09 20

A simple reproduction

The following is a reproduction using apollo-boost. The same problem occurs using apollo-link-error directly.

import ApolloClient from 'apollo-boost';

const client = ApolloClient({
  uri: '/my_api',
  onError: (({ response, networkError }) => {
    if (networkError && networkError.statusCode === 401) {
      console.log(response); // prints 'undefined'
      response.errors = null; // will throw 'undefined' error
      response = { errors: null }; // will throw a 'read-only' error 
    }
  })
});

Issue Labels

alfonmga commented 6 years ago

Is there any temporary workaround for this bug?

vnenkpet commented 5 years ago

+1 a pretty big problem for debugging if you're using apollo client in server environment (reading data from a different GraphQL server for your resolvers).

williangd commented 5 years ago

+1 Is there any workaround to conditionally ignore errors?

escorponox commented 5 years ago

I also have this issue.

The big problem to me is that this prevents the UI to be aware of this error. It remains on loading state when there is a big error actually.

JoviDeCroock commented 5 years ago

I have looked into this and according to the docs, response should be given to the callback: https://www.apollographql.com/docs/link/links/error.html#callback

With the normal callback this happens: https://github.com/apollographql/apollo-link/blob/master/packages/apollo-link-error/src/index.ts#L46

But with the networkError: https://github.com/apollographql/apollo-link/blob/master/packages/apollo-link-error/src/index.ts#L62

marhub commented 5 years ago

any insight on when it will be merged ? :)

JoviDeCroock commented 5 years ago

Probably quite soon, the apollo team is focussed on releasing the new client and react version.

dljcollette commented 5 years ago

Any progress on this issue?

joshnabbott commented 5 years ago

I'm also wondering if there's any progress on this? Or a workaround in the meantime? I'd like to have a way to propagate custom error messages from the API into the UI.

luki215 commented 5 years ago

I was also facing the same issue. Workaround for me was to return empty Observable.

I just wanted to quietly redirect to maintanace page in case the server responsds 503 - without throwing exception. Here's my code, maybe it'll help someone:

import { Observable } from 'apollo-link';

...

onError(({ networkError }: any) => {
      if (networkError && networkError.status === 503) {
        this.redirectService.goToMaintenance();
        return Observable.of();
      }
});

...
Sceat commented 5 years ago

@luki215 i don't have any error code field image

luki215 commented 5 years ago

@Sceat which apollo-link do you use? I use apollo-angular-link-http, so that might be the difference.

Sceat commented 5 years ago

@luki215 it was actually my fault, using AWS api gateway i did not properly configured the CORS response for anything else that 200 codes

for peoples running into the issue you can add on api gateway responses in default 4xx: image

or in serverless.yml


GatewayDefault4XX:
  Type: 'AWS::ApiGateway::GatewayResponse'
  Properties:
    ResponseParameters:
      gatewayresponse.header.Access-Control-Allow-Origin: "'*'"
      gatewayresponse.header.Access-Control-Allow-Headers: "'*'"
    ResponseType: DEFAULT_4XX
    RestApiId:
      Ref: 'ApiGatewayRestApi'```
strass commented 5 years ago

I believe I'm hitting the same stumbling point and am unable to forward messages into my react application to create error messages with. Is there any progress on a fix for this issue?

Sceat commented 5 years ago

@strass dunno about this response field but to handle errors you can switch on network error

export default onError(({ graphQLErrors, networkError }) => {
    if (graphQLErrors) {
        for (let { extensions: { code } } of graphQLErrors)
            handleTheError(code)
    }
    if (networkError) {
        switch (networkError.statusCode) {
            case 500:
                //
                break
            case 429:
                //
                break
            case undefined:
                //
                break
            default:
                console.error(networkError)
                break
        }
    }
})
alfondotnet commented 5 years ago

Just tried @luki215 's workaround of returning an empty Observable but in my case that will now make react-apollo throw an exception:

Uncaught (in promise) TypeError: Cannot read property 'data' of undefined
    at Mutation._this.onMutationCompleted (react-apollo.esm.js:627)
    at react-apollo.esm.js:564
joshnabbott commented 5 years ago

I'm using this to mutate the error returned to the component using the Mutation

const errorLink = onError(({ forward, networkError, operation }) => {
  if (networkError) {
    const { message: errorMessage } = networkError.result;
    switch (networkError.statusCode) {
      case 400:
        if (errorMessage) {
          networkError.message = errorMessage;
        }
        break;
      case 403:
        window.location = '/users/login';
        break;
      default:
        break;
    }
  }
  forward(operation);
});

I think the key thing people are maybe looking for here is the ability to override the error string returned in networkError and it's done with this networkError.message = 'Some custom error message.'

kalzoo commented 5 years ago

Came up with a hacky workaround after digging into apollo-link-rest and this package. Rather than using onError, I rewrote my own (below).

The intent: intercept a NetworkError and conditionally transform it to a GraphQLError. Reason being, I'm using a REST endpoint that doesn't follow GraphQL convention, and returns shapes like {"message": "{ \"key\": \"not valid\"}"} with a 400 status code. That shouldn't be treated as a NetworkError.

What works: this solution does pass down the actual error message from the response down the chain, in a way that I can retrieve and parse it within the mutate promise catch and modify the child form's state.

What doesn't work:

const errorLink = new ApolloLink((operation, forward) => {
    return new Observable(observer => {
      let sub: ZenObservable.Subscription;

      try {
        sub = forward!(operation).subscribe({
          next: result => {
            console.log("A proper result looks like:", result);
            observer.next(result)
          },
          error: networkError => {
            try {
              const { message } = networkError.result;
              const error = new GraphQLError(message);
              const apolloError = new ApolloError({
                errorMessage: "Some error happened",
                graphQLErrors: [error],
                networkError: undefined,
              });

              observer.error(new Error(message));
            } catch (_) {
              observer.error(networkError)
            }
          },
          complete: () => {
            console.log("Calling complete()");
            observer.complete();
          },
        });
      } catch (e) {
        console.log("ErrorLink had an error of its own")
        observer.error(e);
      }

      return () => {
        if (sub) sub.unsubscribe();
      };
    });
  });

It seems to me like this should work, but the mutate() function doesn't seem to resolve or reject - neither then() nor catch() is called.

const errorLink = new ApolloLink((operation, forward) => {
    return new Observable(observer => {
      let sub: ZenObservable.Subscription;

      try {
        sub = forward!(operation).subscribe({
          next: result => {
            console.log("A proper result looks like:", result);
            observer.next(result)
          },
          error: networkError => {
            try {
              const { message } = networkError.result;
              const error = new GraphQLError(message);

              // this is called, but it seems to be the end of the line
              console.log("Intercepting error, returning success");
              observer.next({ data: {}, errors: [ error ]});
            } catch (localError) {
              console.error("Error in link:", localError);
              observer.error(networkError)
            }
          },
          complete: () => {
            console.log("Calling complete()");
            observer.complete();
          },
        });
      } catch (e) {
        console.log("ErrorLink had an error of its own")
        observer.error(e);
      }

      return () => {
        if (sub) sub.unsubscribe();
      };
    });
  });
PassionateDeveloper86 commented 5 years ago

Just pushing... Stumbling on the same error now. Want to reset the error in the response field to handle the error on the component itself...

Any progress?

mluis commented 5 years ago

+1 on the same issue.

strass commented 5 years ago

I think I might have found a solution (at least to my error propagation issue):

Replacing map with forEach in errorLink means errors can propagate errors to UI (previously result.error in UI component was undefined, now it has the error(s)):

const link = onError(({ graphQLErrors, networkError }) => {
  if (graphQLErrors)
-    graphQLErrors.map(({ message, locations, path }) =>
+    graphQLErrors.forEach(({ message, locations, path }) =>
      console.log(
        `[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}`,
      ),
    );

  if (networkError) console.log(`[Network error]: ${networkError}`);
});
strongpauly commented 5 years ago

I was also facing the same issue. Workaround for me was to return empty Observable.

I just wanted to quietly redirect to maintanace page in case the server responsds 503 - without throwing exception. Here's my code, maybe it'll help someone:

import { Observable } from 'apollo-link';

...

onError(({ networkError }: any) => {
      if (networkError && networkError.status === 503) {
        this.redirectService.goToMaintenance();
        return Observable.of();
      }
});

...

This has worked for me, but it's doing exactly the opposite of what the documentation says, i.e. "The error callback can optionally return an observable from calling forward(operation) if it wants to retry the request. It should not return anything else."

strass commented 5 years ago

After updating Apollo today my solution above no longer carries graphQL errors through apollo-hooks to the UI.

Does anyone have another solution?

krvajal commented 4 years ago

@strass can you explain why your solution above fixes the issue. IMO they do exactly the same thing using the forEach and the map.

strass commented 4 years ago

It doesn't any more (see https://github.com/apollographql/apollo-link/issues/855#issuecomment-538010335 )

svalchinov commented 4 years ago

Any progress on this?

How are we meant to deal with a case where the server is returning a 302 with a "Location" header to re-authenticate, if we can't get the response object at all?

If someone has any clue do let me know cos we're thinking of dropping apollo :(

svalchinov commented 4 years ago

This helped me massively https://github.com/apollographql/apollo-link/issues/297#issuecomment-350488527 and allows network responses to be read.

Something which is crucial when there are unauthorised responses returned from authentication layers/gateway APIs.

I'd love to see this as part of the core functionality!

yifengd commented 4 years ago

Any progress on this? There is no way this is the intended behavior. How else can one forward GraphQL errors to the client with a non-200 status code?

arimus commented 4 years ago

Just a note for anyone else running into an issue with catching and modifying network errors. I was trying to unwrap network errors to the inner error from graphql and here's what I ended up with:

const errorHandler = onError(({ graphQLErrors, networkError, operation, forward, response }) => {
    if (graphQLErrors) {
      console.error('apollo errors', graphQLErrors);
    }
    if (networkError) {
      console.error('apollo network errors', networkError);
      if (!!networkError['error'] && !!networkError['error']['errors'] && networkError['error']['errors'][0]) {
        console.error('unwrapping apollo network errors');
        networkError.message = networkError['error']['errors'][0].message;
        // you may also be able to set networkError.message to null based on criteria to remove the error, even if you can't prevent an error from being triggered altogether
      }
    }
  }
);

// Create an http link:
const httpLink = new HttpLink(this.httpClient).create({
  uri: this.uri,
  headers: new HttpHeaders({
    Authorization: this.token
  }),
  includeExtensions: true
});

const httpErrorLink = ApolloLink.from([
  errorHandler,
  httpLink,
]);

Essentially I replace the message which is a generic network error message with the first internal error message. Might help some folks like me that ended up here.