Handle partial responses when the server returns an error

[stubailo]

XXX in QueryManager#makeRequest.

[stubailo]

Still haven't fixed this

[stubailo]

Interesting news: Relay doesn't do anything about this. They just ignore the error and put the incorrect data into the store. Perhaps this is because the Facebook backend doesn't return errors like that?

[martijnwalraven]

Hmmm, so if the server sends a partial response plus errors, you just receive the partial response?

Do you know how they deal with #106?

[stubailo]

They just overwrite the store with 'null' so you can't actually tell if something is missing due to an error or if it's actually missing. I asked them about this specifically.

[stubailo]

Marking this as low priority because Relay doesn't do this, and GraphQL-JS doesn't even have a way to get this information.

[BartKrol]

I've started playing with apollo-client and run into a problem which I guess it's already discussed here (not sure though - I didn't want to open another issue :P)

I'm making a following query to my graphql server with GraphqiQL:

{
  team {
    name,
    members {
      name
    }
  }
}

As a result I'm getting some data and an Error, because one team member had an incorrect id. It's not ideal, but I consider this as a valid response. Unfortunately when I make the same query with apollo-client I'm getting only a list of ApolloErrors. In my opinion this is an incorrect behaviour.

[stubailo]

In my opinion this is an incorrect behaviour.

Relay will do something similar - basically, since errors don't come with a reliable location there is no way to know which part of the response is reliable. This is something we're actively trying to get fixed in GraphQL itself.

But perhaps we could add an option to be a bit looser with error handling.

[beepboopitschloe]

+1 for a config option. I would like to be able to deal with incorrect data myself while the proper GraphQL solution is being developed.

[stubailo]

So here's where that gets handled: https://github.com/apollostack/apollo-client/blob/10cfa7962e58f226573ae6eb63f30c2ce528d7ea/src/QueryManager.ts#L273

What should we do instead?

[beepboopitschloe]

@stubailo Thanks for the context-- I'll take a look tonight and give you my thoughts tomorrow.

[zackzackzackzack]

👍 For this to.
Being able to use the errors to decide what to do with the data that is returned will be helpful.

[beepboopitschloe]

At a high level, what would be ideal for our use case would be an option passed to ApolloClient:

const client = new ApolloClient({
  partialResultsOnError: true
})

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:

{
  data: [{
    team: {
      members: [{
        id: 'b',
        name: 'Foo Bar'
      }, null, null]
  }],
  errors: [ Error(could not find id 'a'), Error(could not find other id 'c') ]
}

If partialResultsOnError is false, 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.

[stubailo]

So here's the long and short of the problem.

1. Apollo uses a global cache to store all of the data you have ever fetched, which is important if you want your UI to be consistent, or you need to handle mutation results and have them show up properly.
2. GraphQL.js simply returns 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:

1. You fetch query A, the result is cached
2. You fetch query A again, but this time there's an error that caused a null, the result is cached
3. Now, you have a null 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?

[BartKrol]

I don't know how caching is done in apollo, but in the example I've shown I'd like to render as much as I can for my UI. When making a request for a team and team.members, I'd like to show all the information about a team and use data about those people I could get from team.members. With current state when one members fails I cannot use any of the data.

With this being said I think it's a good approach to put all of the data into the cache even if it's invalid or at least make an option to mark queries which might fail (and put them always in a cache).

SideNote Regarding the second point:

GraphQL.js simply returns null on any field where there is an error, and doesn't have any indication of where in the response the error happened.

For this example query

{
  team {
    name,
    members {
      name
    }
  }
}

I'm getting:

...
    {
      "message": "Error: cannot GET http://peopleapi.com/people/testID (404)",
      "locations": [
        {
          "line": 5,
          "column": 6
        }
      ]
    },

Which, as far as I understand, will show where error happened in the query. Of course it doesn't tell everything, because for example it will not tell which member exactly it was, but it might be interesting to use it to invalidate only a part of a query by apollo.

[stubailo]

We could take advantage of this PR we added to graphql-js, to know exactly where the error happened: graphql/graphql-js#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?

[beepboopitschloe]

Per-query is probably more user-friendly as it gives the finest level of control. But maybe there could be a global option as well which would provide a default. I'm suggesting this because personally I would like to turn on a global option and not worry about it, but other people might need to cache more aggressively or maintain functionality without a network connection.

Is there currently a pattern for per-query options with a global default?

[stubailo]

No - let's add the thing that you need, and if someone comes along and needs per-query control then we'll add it later. I'd rather not add hypothetical features.

[beepboopitschloe]

Sounds good. One more question: should this behavior apply to mutations as well? We're not using mutations yet so I'm less familiar with that area.

[tmickel]

@helfer @stubailo I'm still learning about this problem (and Apollo), but:

In an imaginary Apollo configuration where partial results and errors are returned, could these be cached, too? I.e., a cached response would indicate errors (and partial data), which would then be overwritten (errors cleared) if a new, successful response comes back from the server?

In react-apollo, errors could maybe even be passed to a component as a prop in this mode?

P.s. thank you for Apollo!!!! :)

[sorenbs]

I have been living in blissful ignorance for the last 6 months thinking this had already been fixed :-)

The GraphQL spec is pretty explicit about supporting partial responses and the errors field containing detailed information.

As the most popular GraphQL client I would expect Apollo to have a strong story around error handling.

@helfer @stubailo - what can Graphcool do to help move this forward?

[helfer]

@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:

1. Should errors (with/without path) be written to the store?
2. Should errors be retrieved from the cache if an earlier query errored on the same path?
3. What should happen with queries that have errors without path?
4. How should GraphQL errors be delivered to components? How about non-GraphQL errors?
5. If we don't cache errors, how should we deal with them during SSR?

There are probably a few more missing here, but

[sorenbs]

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.

[thifranc]

Hi !

@helfer thanks a lot for all the work you do, that's amazing !
Do you have any leads about when the release of the "error handler feature" for apollo-client is due ?
And moreover, we're currently implementing an error handling system following this article.

If we return something like :

{
  errors: [{
    message: 'Request is invalid.',
    path: ['createUser'],
    myCustomErrorData: {
      '': ['Failed to create a new user.'],
      email: ['A user with this email already exists.']
    },
    ...
  }]
}

Do you think that 'myCustomErrorData' will be accessible with the apollo-client's latest release ?

Cheers,

[cesarsolorzano]

@thifranc you could use response.graphQLErrors[0].myCustomErrorData to access the data.

[stale]

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!

[stale]

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!

[oliverjam]

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.

query eventQuery {
  transaction(id: 111111) {
    performance(siteId: 1) {
      venue {
        name
        town
      }
    }
  }
}

We get this response:

{
  "data": {
    "transaction": {
      "performance": {
        "venue": {
          "name": "Kobetamendi",
          "town": null,
        }
      }
    }
  },
  "errors": [
    {
      "message": "Error trying to resolve town.",
      "locations": [
        {
          "line": 9,
          "column": 11
        }
      ]
    }
  ]
}

Apollo doesn't pass any of the partial data it gets right now, which is frustrating.

[larryranches]

@helfer Is there any traction on this?

[vidyakhadsareibm]

@oliverjamI am facing the exact same issue you mentioned. @stubailo do you have any updates?

[stale]

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!

[stubailo]

@jbaxleyiii has this been resolved with the new error policies? BTW we might want to add a message to the bot that says you can make it not stale by commenting

[vidyakhadsareibm]

@jbaxleyiii please let us know if this is fixed. Its affecting my design decisions.

[bl42]

https://www.apollographql.com/docs/react/basics/queries.html#graphql-config-options-errorPolicy

errorPolicy has fixed all my issues.

[tmickel]

awesome!!!!!! thank you!!

[AndrewHenderson]

@bl42 I am finding that all results in the error being omitted from the props method on the graphql callback object.

graphql(query, {
    options: ownProps => ({
      errorPolicy: 'all',
      variables: {
        personId: ownProps.match.params.id,
        startDateTime: ownProps.startDateTime,
        endDateTime: ownProps.endDateTime,
      }
    }),
    props: ({ data: { error, loading, user } }) => {
      return {
        error,
        loading,
        ...user,
      };
    }

If I omit the error policy, thus defaulting to none, the error is passed in the props 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? 😕

Using the all policy is the best way to notify your users of potential issues while still showing as much data as possible from your server. It saves both data and errors into the Apollo Cache so your UI can use them. (Source)

[liviudobrea]

I'm getting the same unexpected behaviour as @AndrewHenderson with using errorPolicy: all.
The network request contains both data and errors properties, but somehow react-apollo skips errors when merging the result passed to the component props.
However, I am able to catch said errors via middleware.
One (innapropiate) alternative I found working for now is manually requesting the query and processing the callbacks:

this.props.client.query({
        query: getVendor,
	variables: { id },
	errorPolicy: 'all'
}).then(({ data, errors }) => {
	// All network data is available here, but not assigned to the component props
        // when using the graphql decorator on the component instead
})

[hwillson]

Hi all - a lot has changed since this issue was first created. Apollo Client now has much better error handling capabilities. I'll close this issue for now since most of the comments here are outdated, but please open up new issues if you run into further error handling problems with current day versions of apollo-client. Thanks!