Skip to content
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

Improve error handling #345

Merged
merged 2 commits into from Jun 14, 2016

Conversation

@nickuraltsev
Copy link
Member

commented Jun 13, 2016

This PR fixes #24 and #242.

Breaking change: Promise is now rejected with an Error rather than the response object.

@@ -13,6 +15,8 @@ module.exports = function settle(resolve, reject, response) {
if (!response.status || !validateStatus || validateStatus(response.status)) {
resolve(response);
} else {
reject(response);
var error = createError('Request failed with status code ' + response.status, response.config);
error.response = response;

This comment has been minimized.

Copy link
@mzabriskie

mzabriskie Jun 14, 2016

Member

I would change this to pass response in as 4th arg to createError.

@coveralls

This comment has been minimized.

Copy link

commented Jun 14, 2016

Coverage Status

Coverage increased (+1.0%) to 92.8% when pulling 6f2186d on nickuraltsev:errors2 into 120e8f5 on mzabriskie:master.

@mzabriskie mzabriskie merged commit f2c554e into axios:master Jun 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@romainneutron

This comment has been minimized.

Copy link

commented Jun 14, 2016

This would have required a note in the UPGRADE_GUIDE.md file

@misund

This comment has been minimized.

Copy link

commented Jul 15, 2016

Breaking changes should mean a major version change.

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

@misund once we hit 1.0 this will definitely be the case. Until then as the API becomes more hardened, breaking changes will be released as new minor versions. There is a note in the README regarding this https://github.com/mzabriskie/axios#semver.

@romainneutron

This comment has been minimized.

Copy link

commented Jul 18, 2016

Thanks

@woodb

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2016

Could you please note that this is a breaking change in the CHANGELOG?

I've opened PR #416 in an attempt to add this note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.