Skip to content
This repository has been archived by the owner on Nov 11, 2023. It is now read-only.

Improve the error layer #30

Merged
merged 6 commits into from
Aug 27, 2018
Merged

Improve the error layer #30

merged 6 commits into from
Aug 27, 2018

Conversation

fabien0102
Copy link
Contributor

Why

Sometime, in the beautiful nginx world, we can have this kind of non-sense response:

statusCode: 200
content-type: application/json
body: <html />

That result with a proper explosion because we trying to parse html as json.

This PR add an extra layer of security for this kind of cases -> "never trust server response" 😈

@fabien0102 fabien0102 added the enhancement New feature or request label Aug 24, 2018
@fabien0102 fabien0102 self-assigned this Aug 24, 2018
@fabien0102 fabien0102 requested a review from TejasQ August 24, 2018 15:35
TejasQ
TejasQ previously requested changes Aug 24, 2018
Copy link
Contributor

@TejasQ TejasQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the usage of the isError flag is a little bit of an anti-pattern: the control flow is described using async/await mimicking synchronous flow, yet there is await processResponse().catch here.

Consider wrapping this in a try/catch instead. This would also allow you to remove the isError flag and the mutation thereof.

@fabien0102 fabien0102 dismissed TejasQ’s stale review August 24, 2018 16:11

Updated to the async/await style

@TejasQ TejasQ merged commit 5f33c8c into next Aug 27, 2018
@TejasQ TejasQ deleted the error-handler branch August 27, 2018 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants