Add Clarity around Http Link Errors #244

evans · 2017-11-14T00:53:10Z

We add the result to network and data errors along with documenting errors in the http link's README. This error documentation used to happen in Apollo Fetch and since we are no longer using it the behavior should be present here.

This modifies the error types in a backwards incompatible manner, removing the nullable fields from a client parse error.

The PR also adds a couple changes to the docs README to detail how to run them locally.

If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
Make sure all of the significant new logic is covered by tests
Rebase your changes on master so that they can be merged easily
Make sure all tests and linter rules pass
Update CHANGELOG.md with your change
Add your name and email to the AUTHORS file (optional)
If this was a change that affects the external API, update the docs and post a link to the PR in the discussion

evans · 2017-11-14T00:53:54Z

I wasn't sure if I needed to bump the doc package.json, so I can if that is necessary and then document it in the docs README

apollo-cla · 2017-11-14T00:56:34Z

	Messages
📖	Please add your name and email to the AUTHORS file (optional)
📖	If this was a change that affects the external API, please update the docs and post a link to the PR in the discussion

Generated by 🚫 dangerJS

gpoitch · 2017-11-14T02:17:56Z

With these changes, you could also add graphQLErrors: networkError.result.errors to the errorHandler here:

apollo-link/packages/apollo-link-error/src/index.ts

Lines 29 to 35 in 03dfb41

    
           error: networkError => { 
        
             errorHandler({ 
        
               operation, 
        
               networkError, 
        
             }); 
        
             observer.error(networkError); 
        
           },

peggyrayzis

Thanks for this PR, it looks great! I had one minor comment, but overall this is good to merge. 😎

peggyrayzis · 2017-11-14T02:12:23Z

docs/README.md


 ```
-git clone --recursive https://github.com/apollographql/react-docs.git
-cd react-docs
+git submodule init


Thank you for cleaning up the docs workflow instructions!! 🎉

peggyrayzis · 2017-11-14T02:30:46Z

packages/apollo-link-http/README.md

@@ -69,7 +69,48 @@ client.query({
 })
 ```

-## Upgrading from `apollo-fetch` / `apollo-client` 
+## Errors


Should we mention apollo-link-error in this section as the official way to handle errors?

Great thought!

jbaxleyiii

This is great! A much clearer way of reporting errors! Thank you!

morgansierrasnyder · 2017-11-14T17:49:25Z

packages/apollo-link-http/src/httpLink.ts

+      parseError.statusCode = response.status;
+
+      throw parseError;
+    })


exactly what I needed ^ ! thanks so much for these changes @evans

stubailo · 2017-11-14T18:08:05Z

packages/apollo-link-http/README.md

+| GraphQL Error |    `next`   | `` |
+
+```js
+type ServerError = Error & {


I'd prefer something less oriented towards Flow/TS users here - maybe some example error objects?

stubailo · 2017-11-14T18:08:35Z

packages/apollo-link-http/README.md

+};
+```
+
+## Upgrading from `apollo-fetch` / `apollo-client`


I think apollo-client in this heading is a bit misleading - maybe createNetworkInterface?

stubailo · 2017-11-14T18:09:52Z

packages/apollo-link-http/README.md

+This list describes the different errors and what the response Observable calls:
+
+* Client parse errors: the request body is not-serializable due to circular references for example
+* Server parse errors: the response from the server cannot be parsed((response.json())[https://developer.mozilla.org/en-US/docs/Web/API/Body/json])


I think this link is mis-formatted - probably worthwhile to look at the deploy preview or run locally

… errors

… and formatting

…README

evans · 2017-11-17T06:23:20Z

@gpoitch That is an incredible thought! I added the alias in.

@peggyrayzis @jbaxleyiii I'm happy with the state of things. Let me know if you see anything else that needs attention.

maurodelazeri · 2017-11-21T15:01:04Z

Amazing!!! ty guys

TomPridham · 2017-11-21T21:47:12Z

is there an ETA for this to get merged and released?

evans requested review from jbaxleyiii and peggyrayzis November 14, 2017 00:53

evans mentioned this pull request Nov 14, 2017

ErrorLink: unable to get graphQLErrors #208

Closed

peggyrayzis approved these changes Nov 14, 2017

View reviewed changes

jbaxleyiii approved these changes Nov 14, 2017

View reviewed changes

morgansierrasnyder reviewed Nov 14, 2017

View reviewed changes

stubailo reviewed Nov 14, 2017

View reviewed changes

Evans Hauser added 5 commits November 17, 2017 00:06

add more information about npm run bootstrap to README

85cd859

add instructions to README for testing the docs locally

5f981c5

Add result to network and data errors with additional types for parse…

cf17dd6

… errors

Add error section to HTTP documentation

64a899a

add apollo-link-error, fixed up links, code snippet more js friendly,…

c06fe47

… and formatting

evans force-pushed the http-error-changes branch from 03dfb41 to c06fe47 Compare November 17, 2017 05:12

Evans Hauser added 2 commits November 17, 2017 00:51

add a note about networkError vs graphQLError in apollo-link-error's …

264e7c9

…README

add graphQLErrors as alias for networkError.result.errors when present

de3f7a7

Changelog reflects error handling modifications

879ec67

jbaxleyiii mentioned this pull request Nov 21, 2017

error link does not give a real error object, thus no networkError.statusCode access possible #218

Closed

Merge branch 'master' into http-error-changes

065e33f

oliviertassinari approved these changes Nov 22, 2017

View reviewed changes

jbaxleyiii merged commit 76ee376 into master Nov 27, 2017

jbaxleyiii deleted the http-error-changes branch November 27, 2017 13:46

evans mentioned this pull request Nov 27, 2017

apollo-link-http loses contents of error responses from apollo-server #178

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Clarity around Http Link Errors #244

Add Clarity around Http Link Errors #244

evans commented Nov 14, 2017 •

edited

evans commented Nov 14, 2017

apollo-cla commented Nov 14, 2017 •

edited

gpoitch commented Nov 14, 2017

peggyrayzis left a comment

peggyrayzis Nov 14, 2017

peggyrayzis Nov 14, 2017

evans Nov 17, 2017

jbaxleyiii left a comment

morgansierrasnyder Nov 14, 2017

stubailo Nov 14, 2017

stubailo Nov 14, 2017

stubailo Nov 14, 2017

evans commented Nov 17, 2017

maurodelazeri commented Nov 21, 2017

TomPridham commented Nov 21, 2017

Add Clarity around Http Link Errors #244

Add Clarity around Http Link Errors #244

Conversation

evans commented Nov 14, 2017 • edited

evans commented Nov 14, 2017

apollo-cla commented Nov 14, 2017 • edited

gpoitch commented Nov 14, 2017

peggyrayzis left a comment

Choose a reason for hiding this comment

peggyrayzis Nov 14, 2017

Choose a reason for hiding this comment

peggyrayzis Nov 14, 2017

Choose a reason for hiding this comment

evans Nov 17, 2017

Choose a reason for hiding this comment

jbaxleyiii left a comment

Choose a reason for hiding this comment

morgansierrasnyder Nov 14, 2017

Choose a reason for hiding this comment

stubailo Nov 14, 2017

Choose a reason for hiding this comment

stubailo Nov 14, 2017

Choose a reason for hiding this comment

stubailo Nov 14, 2017

Choose a reason for hiding this comment

evans commented Nov 17, 2017

maurodelazeri commented Nov 21, 2017

TomPridham commented Nov 21, 2017

evans commented Nov 14, 2017 •

edited

apollo-cla commented Nov 14, 2017 •

edited