Fixing #211 #307
Fixing #211 #307
Conversation
| (response.status >= 200 && response.status < 300 ? | ||
| var minCode = 200; | ||
| var maxCode = 300; | ||
| if (response.config.hasOwnProperty('maxRedirects')) { |
There was a problem hiding this comment.
Why change the maxCode? Anything in the 300 range should be followed, unless maxRedirects occurs, in which case I assume that would be considered an error. Thoughts?
There was a problem hiding this comment.
I agree with @mzabriskie . On a side note, I'm going to submit a PR that adds support for a config option that allows specifying status codes for which an error should be thrown.
|
@dublx Could you please update README.md? |
|
Hi, I'm not too experienced with PR's.. do I update README and then submit new one? |
|
You can just push a new commit to the same branch. It will be automatically added to your PR. As for max code, my suggestion is to not change the status code error range at all. If you don't want axios to reject the promise for 3XX responses, you will be able to do that via |
|
@dublx Please let me know if you need any help |
|
@nickuraltsev Hi, I've really enjoyed using axios in the chatbot I'm creating for my company. I need this PR to support some needed functionality and currently plan to fork my own copy until this is available. If it is ok with @dublx I would be comfortable with cleaning this branch up to not change the status code error range, update the README and fix any conflicts. Alternatively I could simply create a new PR. In short, I'd like to see the work I'm about to do be put back into axios to share with others and so I don't have to maintain my own fork. Please let me know if I can be assistance. |
|
@nickuraltsev Will do! |
|
Hi guys @dublx @nickuraltsev @cspray @mzabriskie, is there a chance that this PR would be merged soon? I'm really looking forward to it :) Can I help with it somehow? |
|
Merged |
Fixing #211