Skip to content
This repository has been archived by the owner on Jan 20, 2020. It is now read-only.

Fix request error handling #210

Merged
merged 3 commits into from
Jan 18, 2018
Merged

Conversation

rmm5t
Copy link
Contributor

@rmm5t rmm5t commented Jan 8, 2018

What does it do?

Related Issues

itajaja and others added 2 commits January 8, 2018 11:47
Ensures that non-2xx response codes return an error
@rmm5t
Copy link
Contributor Author

rmm5t commented Jan 8, 2018

/cc @fb55 This one is kind of important, IMO.

Copy link

@itajaja itajaja left a comment

Choose a reason for hiding this comment

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

looks great

}
if (data === null) {
} else if (response.statusCode > 299) {
err = new Error(`HTTP ${response.statusCode} Error`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the message from the response here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the statusMessage from the response, or the message from the body, or both? I'm inclined to try both.

Copy link
Contributor

Choose a reason for hiding this comment

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

message from the body. statusMessage wouldn't be too bad either 🤷‍♂️

@rmm5t
Copy link
Contributor Author

rmm5t commented Jan 18, 2018

@fb55 I added the body's error message to the Error object's message. I skipped the statusMessage, because it's not all that beneficial anyway and nock doesn't yet support mocking it at the moment.

@fb55 fb55 merged commit ba94d6e into coinbase:master Jan 18, 2018
@fb55
Copy link
Contributor

fb55 commented Jan 18, 2018

Amazing, thanks a lot!

@rmm5t rmm5t deleted the 208-fix-request-errors branch January 18, 2018 16:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

APIs requests don't error out
3 participants