apollo-client: Handle partial responses when the server returns an error
XXX
in QueryManager#makeRequest
.
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Reactions: 2
- Comments: 50 (21 by maintainers)
Commits related to this issue
- #51: Handle partial responses when the server returns an error — committed to zackzackzackzack/apollo-client by deleted user 8 years ago
- Merge pull request #51 from TimMikeladze/patch-1 Fix syntax error — committed to apollographql/apollo-client by deleted user 8 years ago
This issue has been automatically closed because it has not had recent activity after being marked as stale. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to Apollo Client!
Can we re-open this? The problem still exists—I just hit it today.
Querying event data means that for some languages e.g. a town might not exist and returns null.
We get this response:
Apollo doesn’t pass any of the partial data it gets right now, which is frustrating.
https://www.apollographql.com/docs/react/basics/queries.html#graphql-config-options-errorPolicy
errorPolicy has fixed all my issues.
At a high level, what would be ideal for our use case would be an option passed to
ApolloClient
:When this option is set, and a query contains errors, the client would resolve successfully with what information it did receive and attach an
errors
field:If
partialResultsOnError
isfalse
, the client would act exactly as it does now.How well does this fit into the existing system? I’m happy to make a PR if you can give me some pointers on where to start.
This issue has been automatically marked as stale becuase it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!
@sorenbs this is currently blocked on the store refactor which we’re working on at the moment. Currently the plan for the new API is to handle errors the same way as we handle data, and write them into the store as well if they have a path (which means you might retrieve cached errors from the store). I’m not yet 100% clear on what we should do with errors that don’t have a path, but my intuition is to give people a lot of control around how they want to handle those, and decide on a per-query basis whether something with errors should be written to the store or returned from the store. This will make it very easy to configure Apollo Client in just the way people need, but it comes with the drawback that it will be more complicated to tell what’s really going on, which is why choosing sensible defaults will be very important.
What you could help us with is letting us know exactly how you would want your client to behave in different circumstances, and helping us choose sensible defaults so that Apollo Client does what people expect right out of the box (while keeping options open to modify its behavior).
Here are some questions to discuss:
There are probably a few more missing here, but
@stubailo Thanks for the context-- I’ll take a look tonight and give you my thoughts tomorrow.
Thanks Jonas - that’s awesome news!
I very much agree with allowing developers to configure the behaviour, and agree the sensible defaults are super important.
I’ll take some time soon to write up my perspective on this.
So here’s the long and short of the problem.
null
on any field where there is an error, and doesn’t have any indication of where in the response the error happened.So, basically this unfortunate situation can happen:
null
, the result is cachednull
in your cache, even thought you had some good data from (1)Given that there’s no way to tell the difference between a real null result and an error, we decided it’s best to just keep the original data and not let the new result “poison” it.
What’s a different approach that would be better?
For example, we could just always put the data in the cache and deal with the fact that it might be wrong; we could have a configuration option that would let you look at the error and say whether the result should be used or not; maybe something else?
@bl42 I am finding that
all
results in the error being omitted from theprops
method on thegraphql
callback object.If I omit the error policy, thus defaulting to
none
, the error is passed in theprops
callback.If I use
ignore
, it is not passed, which is expected.If I use
all
, the error is still not passed.Am I missing something? 😕
@helfer Is there any traction on this?
This would be quite important for a project I’m working on too. Use case (dummy schema):
favouriteBook
needs to be fetched with a resolver for eachCustomer
in the list, via http, as it’s in a separate micro-service. If one of the resolvers fails, meaning that we don’t get a favouriteBook for that customer, apollo client doesn’t return the remaining data, even though it comes in the network response. This means that the entire list fails to display, even though only one of the elements had an error.Now, maybe we can handle this in the resolver by catching the error and returning empty data, however it would still be helpful to see the errors, as taking this approach would completely hide potential problems.
Error handling is high on our list of priorities, but we decided not to do it for 0.5 yet.
We could take advantage of this PR we added to graphql-js, to know exactly where the error happened: https://github.com/graphql/graphql-js/pull/396
But yeah, I’d be OK to add an option to ignore the errors and just put everything into the cache until reporting the error path is part of the GraphQL spec.
Do we want it to be per-query, or global?