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

Don't throw error / catch on 400-level responses #41

Closed
aaronshaf opened this issue Feb 3, 2015 · 17 comments · Fixed by #308
Closed

Don't throw error / catch on 400-level responses #41

aaronshaf opened this issue Feb 3, 2015 · 17 comments · Fixed by #308
Assignees
Milestone

Comments

@aaronshaf
Copy link

Instead, limit the error/catch to 500-level errors. 400-level responses can be completely valid, expected, desired, etc.

I have been following this pattern with my bunyan logging -- logging 400-level responses as warnings, and 500-level responses as errors. The idea is that a successful npm test (which may expect 400-level responses) should show zero logging errors.

@mzabriskie
Copy link
Member

It's hard to make a change like this at this point. Where people expect it to catch on anything outside of the 2xx range. In retrospect, if the request was made, and a response received I would have preferred it to be considered a success. This would be a much less opinionated implementation, and much better IMO. I think the way to handle this now would be to pass a config option that specifies what range to catch on, defaulting with anything outside of 2xx.

@mik01aj
Copy link

mik01aj commented Feb 17, 2015

@mzabriskie +1

@AndrewRayCode
Copy link

I'm confused. Does axios fail the promise on 4xx? This thread makes it sound like it DOES, but in my code i'm getting a resovled promise on status 4xx. It doesn't appear to be documented.

@mbektimirov
Copy link

Same thing for me, 400 (BAD REQUEST) error resolves in then, not in catch. Very annoying bug, breaks all REST flow.

@mik01aj
Copy link

mik01aj commented Jul 22, 2015

The code says that it's resolved when request.status >= 200 && request.status < 300 and rejected in any other case.

https://github.com/mzabriskie/axios/blob/1629a026da17a1e1d8999a02f3fe6b6b60aaac9c/lib/adapters/xhr.js#L58
https://github.com/mzabriskie/axios/blob/db85c7bf3ae19d680f5c16cd85d06c5e11fedc5f/lib/adapters/http.js#L80

@mbektimirov
Copy link

The problem is with response interceptor. It puts error object in then callback instead catch:

axios.interceptors.response.use(response => {
  return response;
}, error => {
  return error;
});

@clayreimann
Copy link

@mbektimirov A little late here, but I'm guessing that you're seeing the error in then because your error callback isn't throwing the error.

@mountHouli
Copy link

mountHouli commented Apr 26, 2016

+1
Using axios for the first time. Definitely like it, but did just run into this; it would be great for the 400s to not throw errors.
Yay! Assigned and in progress as of 4 days ago. Thanks y'all!

@mzabriskie
Copy link
Member

This has been added as a feature in 0.11.0 by use of validateStatus config option.

@adadgio
Copy link

adadgio commented Jul 17, 2017

I totally y agree, it shouldn't throw any error passed 500 errors, which basically prevents doing any REST... I think this is very "bad design" (don't be mistaken, the library is great!!)

How is this resolved pass 0.11.0, any example on how to use the option maybe ? (using 0.16 here)
Is there any way to use this in a classic GET/POST shortcut?

Thanks!

@roberto-formulatrix
Copy link

Agreed, luckily the response is in the error object so you can deal with it, but would make sense to have it in the regular flow. I always felt like 200-400s was more related to http, whereas 500+ were more application errors. E.g. validaation failed, etc.

@AndreiBru
Copy link

(on 0.16.2) - 400 Response still resolves in then instead of catch, and response object is undefined regardless of what server returns.

@carlojacobs
Copy link

carlojacobs commented Dec 12, 2017

I agree, I use 401 response in case of a login failure, in that case I'm also sending a message, but because it passes it to the .catch(), I can't access that data.

Any news on how to fix/modify this?

@jonathanpmartins
Copy link

@carlojacobs you can access error.response.data

axios.post('/').then(response => {
    console.log(response.data);
}).catch(error => {
    console.log(error.response.data);
});

@kerembaydogan
Copy link

kerembaydogan commented May 4, 2018

@mbektimirov After seeing @clayreimann suggestion I have changed my interceptor as below.
Now error resolves in catch block rather than then block.

Hope this helps someone.

axios.interceptors.response.use(function (response) {
  return response;
}, function (error) {
  return Promise.reject(error);
});

@hugosbg
Copy link

hugosbg commented Apr 18, 2019

axios.defaults.validateStatus = function () {
    return true;
};

Or

axios({
    ...,
    validateStatus: function () {
        return true;
    }
});

@lukepighetti
Copy link

I can't think of a compelling reason to have axios to take server errors and throw them as runtime errors. Much preferred to handle the response codes manually, especially since TypeScript doesn't provide types for thrown errors that I'm aware of, so you end up having to go digging through an untyped object to try to figure out what the heck is going on. It also breaks up app flow considerably. Just one mans opinion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.