-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Design docs #923
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
Merged
Merged
Design docs #923
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
036d624
add initial draft for design docs
helfer 9ac80af
add draft for errors proposal
helfer 81e70f6
Merge branch 'master' into design-docs
helfer 19e75c2
Merge branch 'master' into design-docs
helfer 63081e5
fix typo in filename
helfer 5443e1d
Merge branch 'master' into design-docs
helfer b66c6df
Merge branch 'master' into design-docs
helfer b548df7
Merge branch 'master' into design-docs
helfer 8612565
Merge branch 'master' into design-docs
helfer 00eae38
update errors design based on feedback
helfer 6c43b14
Merge branch 'master' into design-docs
helfer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# Apollo Design Documents | ||
|
||
A design document is a way to communicate to others what the intent behind a certain feature/solution is. In the Apollo community we use them to facilitate discussions and reach agreement about important design decisions. | ||
|
||
Design docs are written with the purpose of helping community members and contributors understand design proposals and judge them on their merits. | ||
|
||
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 | ||
- easy to understand and use | ||
- compare the proposed solution with obvious alternatives and show that | ||
- the proposed solution is better than its alternatives with respect to the Apollo vision. | ||
|
||
For an example of a design doc, see [Apollo Errors](./designs/errors.md). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# 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](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: | ||
|
||
1. Errors that make the entire result unusable | ||
2. Errors that make only part of the result unusable | ||
|
||
An example for the first kind of error is when the server cannot be reached, or the server does not return valid JSON. An example for the second kind of error is when the server returned a partial result and a GraphQL error because it could not reach a backend service. | ||
|
||
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.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. | ||
|
||
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. 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.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. | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also add some factors like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like your first point is included in the comparison with alternatives. I'll add the second point.
PS: I actually had a more complete list, but I felt like it was getting too long. I'd say let's start simple and add more stuff later if we see that it's necessary.