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

HTTP Status Code - Helpful on Errors #542

Closed
philcockfield opened this issue Aug 15, 2016 · 13 comments
Closed

HTTP Status Code - Helpful on Errors #542

philcockfield opened this issue Aug 15, 2016 · 13 comments

Comments

@philcockfield
Copy link

Scenario:

Connecting to a GraphQL server that require authentication, and the client is either not authenticated, or the authentication token has expired.

Current Experience:

The current experience is that the Apollo Client throw an error that contains details about failing to parse a JSON response:

Better Experience:

It would be better to be able to see on the error the HTTP status code, the raw response, and anything else from the failed HTTP request that could be used to determine what to do next:

try {
  const { data } = await graphql.query({ query });

} catch (err) {

  console.log("err", err.status);

}

Thanks

@gauravtiwari
Copy link

Hey @philcockfield,

You could use an afterware middleware, http://docs.apollostack.com/apollo-client/index.html#networkInterfaceAfterware

is this what you looking for?

@philcockfield
Copy link
Author

philcockfield commented Aug 15, 2016

Hey @gauravtiwari - yeah, that will do it.

Still feels like it would be good to get status codes on a failed query though. The failure point will be explicitly where the code is calling query, and being able to choose what to do at that call-site is a good place to locate that logic.

afterWare feels like an elegant solution for global, generic problems that come up - like a blanket 401 => logout

Thanks!

@Poincare
Copy link
Contributor

This makes sense to me. Often, different queries require you to handle response codes differently. I'm thinking we could pass the response information returned by fetch into the promise handler or observer. This probably means we'll have to add another field to the ApolloQueryResult structure.

@stubailo, thoughts?

@stubailo
Copy link
Contributor

The current experience is that the Apollo Client throw an error that contains details about failing to parse a JSON response

Would it be sufficient if this error just included the entire HTTP response including headers and response code? Also, we should probably treat any 4** or 5** response as an error.

@philcockfield what status code does your server return when the authentication fails?

@philcockfield
Copy link
Author

philcockfield commented Aug 16, 2016

Would it be sufficient if this error just included the entire HTTP response including headers and response code?

@stubailo - yes, including the entire HTTP response would be exactly the right move here I think - I agree. There's no special abstraction needed, just access to the standard HTTP response data to interpret in edge-cases where you need to.

Also, we should probably treat any 4** or 5** response as an error.

Agree with this also. In that case of say a 401 the HTTP response is included on the Error object. So when using async/await you'd have something like this:

    try {
      const { data } = await graphql.query({ query });

    } catch (err) {

      console.log("HTTP Error Status:", err.response.status);
      console.log("HTTP Error Message:", err.response.statusText);

    }

@philcockfield what status code does your server return when the authentication fails?

In this case it's a 401 which can happen when a JWT token expires.

@helfer
Copy link
Contributor

helfer commented Aug 30, 2016

@philcockfield I'm all for this. Would you be willing to help with a PR for this? I'd help, of course.

@philcockfield
Copy link
Author

philcockfield commented Nov 22, 2016

I might have to get around to doing this. Not having these status codes is beginning to suck! 😄

@crumhorn
Copy link

Please do :) I'm running into this as well on returning a 401. Just having the status code on the NetworkError class would help a whole lot. But also the body of the returned 401 would be nice, as in our case we return JSON along with the 401 to tell the client what exactly the 401 means in this case..

@lukasvan3l
Copy link

I'm also running into this, will go for the afterware solution.
FYI documentation is now at http://dev.apollodata.com/core/network.html#networkInterfaceAfterware

@crumhorn
Copy link

Afterware seems fine for some stuff, but if you need to do Angular2 related calls in your app.module definitions code (where you pretty much have to initialize Angular Apollo) you don't really have access to any of your modules unless you make static calls. It's probably a solution for some stuff, but I still don't understand why the NetworkError class doesn't have the status code and the returned body as part of its fields, which would make it so much easier to do specific logging and calls from the front-end code.

@pseudoramble
Copy link
Contributor

It looks like the networkInterface may need to return a different kind of Error object that would include the response as part of it. I don't think the Error object being thrown from there can supply the response as part of it. Perhaps NetworkError would be good like @crumhorn was suggesting.

I would be willing to try and help with this effort. I'm not familiar with this code though. So I would need some guidance on the design of it. Is there a good place to start for that kind of help?

@cesarsolorzano
Copy link
Contributor

It looks like this should be closed per #1205

@helfer helfer closed this as completed Mar 27, 2017
@k4mr4n
Copy link

k4mr4n commented Jul 15, 2017

in version 1.9.0-0 i'm getting Error: Network error string in catch of query function

ex: () => client.query({
query: gql...
}).then(a => console.log('response =>', a))
.catch(e => console.log('error =>', e))

and i can't use afterware due to using redux-saga (couldn't run generators in afterware)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants