make afterware errors bubble up

[edvinerikson]

This makes errors thrown from afterwares bubble up so that apollo don't try to parse bad responses.

TODO:

• 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

[helfer]

@edvinerikson Thanks for the PR! Can you explain what problem this solves and add a test case if applicable? Right now it doesn't look like your PR would change any behavior.

[stubailo]

Looks like the change makes it so that the promise returns the return value of the afterware. However this should definitely come with a test that ensures the behavior continues to work, especially since it's so easy to miss.

[edvinerikson]

Yeah I will add a test. I just need to fix my local setup first. Can't run it locally due to TS errors.

…

On Fri, 2 Dec 2016 at 23:55, Sashko Stubailo ***@***.***> wrote: Looks like the change makes it so that the promise returns the return value of the afterware. However this should definitely come with a test that ensures the behavior continues to work, especially since it's so easy to miss. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#982 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADtdjIkVxD0gvTIVN8hR7lwS3Y2vvHcNks5rEKHHgaJpZM4LCi3H> .

[edvinerikson]

@helfer the reason for the change is because I have a afterware which I check the response status and decide if Apollo will be able to parse the response. in some cases Apollo won't be able to do that and when that happens I throw a error which I can catch in my error handling but without this change I am not able to catch the error and instead Apollo will fail with when trying to parse the json body.

e.g getDataFromTree(app).catch(error => doSomethingWithErrorHere()) but currently the error I get there will be something like unexpected token < at 1:1 (due to a html response) instead of my own error.

[edvinerikson]

maybe a improvement to the network layer would be to check response.ok before trying to parse / handle the response as well. will probably help fix some of the cryptic error messages that I get sometimes.

currently I do this in my afterware

[helfer]

@edvinerikson Okay. Please add a test, and then we can merge it.

I think checking response.ok won't work, because some people return error codes that are not in the 200 range for responses that should be parsed and processed. It's unfortunate, but I think we have to live with that for now.

[edvinerikson]

I think checking response.ok won't work, because some people return error codes that are not in the 200 range for responses that should be parsed and processed. It's unfortunate, but I think we have to live with that for now.

Yeah I will create my own network layer to handle my cases instead.
Still gonna try to add a test and get this merged.

[edvinerikson]

test is added

[helfer]

Sweet, thanks a lot!