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

Handling error response #150

Closed
4nakin opened this issue Oct 2, 2018 · 17 comments
Closed

Handling error response #150

4nakin opened this issue Oct 2, 2018 · 17 comments

Comments

@4nakin
Copy link

4nakin commented Oct 2, 2018

Hello, I have this Sign-in API which will take request body as:

{
    "authentications": {
        "emailAddress": "[email protected]",
        "password": "11111111"
    }
}

The server return token if success:

{
    "authentications": {
        "token": "eyJhbGoiU3RvcmUifV0sImNyZWF0ZWRBdCI6IjIwMTgtMDktMTZUMTg6NTA6NTYuNT"
    }
}

In case email or password input is incorrect, it will return:

{
    "message": "Cannot find document for class 'User'. searched with parameters: '{\"emailAddress\":\"[email protected]\"}'",
    "errorCode": 1103,
    "status": 404
}

I can successfully sign-in with a GraphQL schema like this:

gql`
  fragment Authentications on any {
    emailAddress: String
    password: String
  }

  fragment AuthInput on REST {
    authentications {
      Authentications
    }
  }

  mutation signIn($input: AuthInput!) {
    authPayload(input: $input) 
      @rest(type: "AuthPayload", path: "/api/v1/authentications", endpoint:"UsersService", method:"POST") {
      authentications @type(name:"Auth"){
        token
      }
    }
  }
`;

My question is how can I get the response in case of error (wrong email/password) ? Because right now if server return error, the response is always null:

Object {
    "data": Object {
        "authPayload": null,
    },
    "errors": undefined,
}
@AleksandrukTad
Copy link

AleksandrukTad commented Oct 3, 2018

If you cannot catch error 400+ I think this issue will be useful.
#151

FYI: for wrong credentials you should return 401.
https://stackoverflow.com/questions/1959947/whats-an-appropriate-http-status-code-to-return-by-a-rest-api-service-for-a-val

@damusnet
Copy link
Contributor

Hi all,

I just ran into this and I too am wondering what is the recommended way to handle a 404. It doesn't trigger the apollo-link-error for instance.

So far I'm thinking of writing a custom error handler link, or check for data content in the response in each <Query /> component.

FWIW I have the same case where a server is returning a 404 instead of a 401 for instance, but changing the server is unfortunately not an option at this point.

I think it would be worth addressing this use case because status codes for REST are very different - and more varied and important - than those from GraphQL.

@SajidQ
Copy link

SajidQ commented Apr 3, 2019

I'm also stuck on this issue. Is there a reason for not returning the full 404 response instead of null?

@williamboman
Copy link

I have yet to run into a case where not treating 404 as an error has helped me, I keep trying to find nice way to work around that fact.

@cmonacaps
Copy link

cmonacaps commented Oct 9, 2019

Is the thinking that 404 Not found could describe a data query that successfully, correctly yields the empty set?

It could also signal that the service owners made such a breaking change that a previously-available endpoint has disappeared.

I think the first case could better be handled by 200 OK along with an empty or null application entity. At least, it seems like these two cases should be readily discernable by the client, as they are very different.

@fbartho
Copy link
Collaborator

fbartho commented Oct 9, 2019

Yes, intentionally empty data sets are a bit more common than one might think? (At least i was surprised!) Since users are often collecting multiple network & other calls into a single meta call, defaulting to throwing an error on 404 has an outsized impact, because one 404 would terminate all other data fetches that happen in parallel. I agree if there was only one REST call per GraphQL query executed, it seemed to me that we should throw an error given the official REST semantics.

In practice not throwing an error by default seemed better! Since you can work around this by adding a custom fetch, and this feels like “be liberal in what you accept”; this choice felt like it matches up with one of the key purposes of Apollo-link-rest: help users that don’t necessarily control the backend, and there are many only “mostly-REST” servers out there, so semantic violations are common.

@fbartho
Copy link
Collaborator

fbartho commented Oct 9, 2019

@williamboman it’s relatively straightforward to configure Apollo-link-rest to throw on 404s if you use the customFetch Parameter I’m not at my computer, but let me know if you need help doing that!

@anasnain
Copy link

Hi @fbartho Can you pls help me with writing customFetch to handle 404 errors? Is there some example on it?

@fbartho
Copy link
Collaborator

fbartho commented Aug 10, 2020

@anasnain:

In javascript it would look something like this:

export async function customFetch(requestInfo, init) {
  const response = await fetch(requestInfo, init);
  if (response.status === 404) {
    throw new Error("404 File Not Found"); // Customize this however you like!
  }
  return response;
}

(In TypeScript it would be a little more verbose)

@anasnain
Copy link

@fbartho Thanks for the quick response. I'm able to catch 404 now.
But how do I make sure networkError of apollo-link-error catch the error (throw new Error("404 File Not Found")) written inside custom fetch?

@fbartho
Copy link
Collaborator

fbartho commented Aug 10, 2020

Ah, unfortunately, I think it will appear attached to the graphQLErrors array (not on the networkError property) due to ApolloClient internal rules (about how links work).

It's possible this is fixable, but I don't know how. -- HTTPLink must know how to do it, and I haven't had the chance to investigate. We have a helper method in our (work) codebase that just unpacks the nested errors. (This was also necessary for errors thrown from apollo-link-state for us so it wasn't really a priority)

@anasnain
Copy link

@fbartho I'm not getting any error at all in this case. data is returning null. was expecting an error instead of data if we are throwing an error. Not sure how to implement this.

@fbartho
Copy link
Collaborator

fbartho commented Aug 11, 2020

@anasnain It's been so long since I set it up, I forgot.

You also need to configure an apollo-link-error instance, this is my configuration:

const attachGQLErrorsToNetworkError = (
	graphQLErrors: readonly GraphQLError[],
	networkError: Error,
): void => {
	(networkError as any).tr_graphQLErrors = graphQLErrors;
	return;
};

/** Set behavior when errors occur and handle our own TR errors */
export const apolloErrorLink = onError((errorResponse: ErrorResponse) => {
	const {
		graphQLErrors = [],
		networkError,
	}: {
		graphQLErrors?: readonly GraphQLError[];
		networkError?: Error;
	} = errorResponse;

	/**
	 * Our error parsing (function recursiveGQLErrorsFromNetworkError) rely
	 * on attaching the following graphql errors.
	 * Let's make sure to not attach anything `wrong` (ie: null, empty, ...)
	 */
	const hasGraphQLErrors: boolean =
		graphQLErrors != null &&
		Array.isArray(graphQLErrors) &&
		graphQLErrors.length > 0;

	if (networkError && hasGraphQLErrors) {
		/*
		 * graphQLErrors are not being passed through the chain of links,
		 * but network errors are attaching the graphQLErrors to networkError
		 * to be able to access them in a component
		 */
		attachGQLErrorsToNetworkError(graphQLErrors, networkError);
	}
});

@fbartho
Copy link
Collaborator

fbartho commented Aug 11, 2020

After further reflection: I'm not entirely sure I understand the situation you're describing.

Is the problem that you cannot read "404 error" in your component/react code?

@anasnain
Copy link

anasnain commented Aug 11, 2020

@fbartho
Scenario: I'm running below rest APIquery and expectation is I should get an error object in case it returns 404 error code just like in case of other 4xx errors and 5xx errors. but in this case, the error is undefined and data is null.
const {loading,error,data} = useQuery(CLIENT_API_QUERY);

This is the Apollo client instance:

async function customFetch(requestInfo, init) {
    const response = await fetch(requestInfo, init);
    if (response.status === 404) {
        response.json().then((errorResponse) => {
          throw new Error(errorResponse);
        });
    }
    return response;
  }

const errorLink = onError(({ operation, response, graphQLErrors, networkError }) => {
    if (graphQLErrors) {
      graphQLErrors.forEach(({ message, path }) =>
           console.log(`[GraphQL error]: Message: ${message}, Path: ${path}`),
      );
    }
    if (networkError) {
        console.log(`[Network error ${operation.operationName}]: ${networkError.message}`);
    }
  });

const restLink = new RestLink({
    uri: 'https://run.mocky.io/v3/df72bacd-39cb-476d-bfda-06d7f5e9d77d',
    customFetch: (requestInfo, init) => customFetch(requestInfo, init)
});

const apolloClient = new ApolloClient({
    link: ApolloLink.from([errorLink, restLink]),
    new InMemoryCache(),
  });
export default apolloClient;

@fbartho
Copy link
Collaborator

fbartho commented Aug 13, 2020

@anasnain -- one thing I'm aware of is that I don't think you can pass an arbitrary object when doing new Error

Also, I think you're missing an await, so the throw statement is happening outside of the restLink customFetch call!

const err = new Error("404 Error");
try {
  err.responseJson = await response.json(); // This await crucial to get the error in the right spot!
} catch (parsingError) {
  // figure out what you want to do with parsingError?
}
throw err;

@fbartho
Copy link
Collaborator

fbartho commented Jan 5, 2022

I'm going to close this ticket since it's been more than a year and there are several proposed workarounds / code-changes.

If you have identified a clear issue or change, please open a new ticket describing just that issue! Thanks!

@fbartho fbartho closed this as completed Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants