Skip to content

Commit

Permalink
update errors design based on feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
helfer committed Nov 29, 2016
1 parent 8612565 commit 00eae38
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
1 change: 1 addition & 0 deletions DESIGNS.md
Expand Up @@ -6,6 +6,7 @@ Design docs are written with the purpose of helping community members and contri

A good design document should:
- state the problem that is being solved as clearly as possible
- explain why the problem should be solved in this package, and not a different one (or possibly a new one)
- show that the proposed solution fits in with the stated vision for Apollo Client
- incrementally adoptable
- universally compatible
Expand Down
12 changes: 7 additions & 5 deletions designs/errors.md
@@ -1,7 +1,7 @@
# Error handling - design proposal

## Motivation
ApolloClient currently does not deal with errors very gracefully. When the server returns any error, Apollo Client will discard the result and call `observer.error`. With the recent addition of error paths in the standard `formatError` function of graphql-js, we have an opportunity to improve error handling and write partial results to the store.
ApolloClient currently does not deal with errors very gracefully. When the server returns any error, Apollo Client will discard the result and call `observer.error`. With the recent addition of error paths in the standard `formatError` function of [graphql-js](https://github.com/graphql/graphql-js/pull/561) and the [proposal to add it to the spec](https://github.com/facebook/graphql/pull/230), we have an opportunity to improve error handling and write partial results to the store when the server provides one.

## Proposed feature / solution
As far as Apollo Client is concerned, errors can be roughly divided into two categories:
Expand All @@ -13,17 +13,19 @@ An example for the first kind of error is when the server cannot be reached, or

For errors of the first category, the server does not return any data that can be displayed to the user, only errors, so Apollo Client should call `observer.error` and not write data to the store.

For errors of the second category, the server returns data *and* errors, so there is data that can be displayed to the user and Apollo Client should call `observer.next` with `{ data, errors }`, and write as much of the result to the store as possible. Any `null` fields in `result.data` should not be written to the store if there is an error with a path corresponding to that field.
For errors of the second category, the server returns data *and* errors, so there is data that can be displayed to the user and Apollo Client should call `observer.error` with `{ data, errors }`, and write as much of the result to the store as possible. Any `null` fields in `result.data` should not be written to the store if there is an error with a path corresponding to that field.

Because initially not all servers will have support for error paths, a configuration option is necessary to tell Apollo Client to use the current default behavior - i.e. call `observer.error` and discard all data.
Note: We call `observer.error` with the partial result instead of `observer.next` in order to make error handling stay as close as possible to the current behavior.

Because initially not all servers will have support for error paths, the current behavior (discarding all data) will be used when errors without path are encountered in the result.

## Implementation steps / changes needed
1. Add a configuration option to ApolloClient to keep using the current error behavior.
1. Call `observer.error` and discard the result if not all errors have an error path
2. Write result to store if at least partial data is available. Ignore fields if there's an error with a path to that field.
3. Call `observer.next` with data and errors if there is at least a partial result.

## Changes to the external API
* ApolloClient will have a new configuration option to keep using the current error behavior. Initially this could be on by default to provide a non-breaking change & smooth transition.
* `observer.next` will now receive errors as well as long as there is a partial result
* `observer.error` will now receive data as well if is a partial result
* Partial results are now written to the store in the presence of GraphQL errors.

0 comments on commit 00eae38

Please sign in to comment.