Adding support for beforeRedirect config option #3852
Conversation
| @@ -273,7 +276,7 @@ module.exports = function httpAdapter(config) { | |||
|
|
|||
| // Handle errors | |||
| req.on('error', function handleRequestError(err) { | |||
| if (req.aborted && err.code !== 'ERR_FR_TOO_MANY_REDIRECTS') return; | |||
| if (req.aborted && err.code !== 'ERR_FR_TOO_MANY_REDIRECTS') reject(err); | |||
There was a problem hiding this comment.
Without this request will never complete.
There was a problem hiding this comment.
just to expand more on this change, I haven't used enhanceError since this should be an error thrown from client application so we return it as is.
It's a bit strange that no one before run into the issue of stuck request
Co-authored-by: Prabodh Meshram <prabodh.meshram7@gmail.com>
|
@zoran995 can you please fix the merge conflicts? |
|
@jasonsaayman merge conflicts are resolved |
|
@jasonsaayman it seems that something weird was going on on my end, typescript never complained about those issues and all tests passed successfully (it started to complain yesterday). Those are totally valid ts errors, which should have been thrown from the start any other ideas? |
|
@jasonsaayman I have resolved type issues, both are now typed as records(objects) |
| @@ -227,6 +227,9 @@ module.exports = function httpAdapter(config) { | |||
| if (config.maxRedirects) { | |||
| options.maxRedirects = config.maxRedirects; | |||
| } | |||
| if (config.beforeRedirect) { | |||
| options.beforeRedirect = config.beforeRedirect; | |||
There was a problem hiding this comment.
Doesn't this break proxy support when redirection occurs?
The setProxy function, which is called before hitting this line, sets options.beforeRedirect to re-apply proxy configuration after a redirect. This line here happily stamps over it.
Either there should be something to harmonize the two features (such as registering a function that invokes both the original config beforeRedirect if needed, and the proxy re-configuration next) or the documentation should state that the config.beforeRedirect option is incompatible with proxy support (😿) and can introduce proxy redirect issues.
Axios for long time supports following of the redirects using the
follow-redirectspackage, but it didn't implemented support forbeforeRedirectfunction which is useful to easily control the redirects.This adds support for providing the
beforeRedirectfunction in options.Also updated the
maxRedirectsdefault value in readme, it is 21 for around 5 years.