Passing timeout as a string is throwing an error in the latest version of axios due to upgrade of follow-redirects

[kanhaiagarwal]

Describe the bug

Axios uses follow-redirects internally for making http requests. When axios was using follow-redirects@1.13.3, we were able to supply timeout in string, e.g. '30000'. Recently, follow-redirects upgraded to v1.14.0. Upon supplying the timeout as a string, eg. '30000', follow-redirects@1.14.0 is throwing the following error:

ERROR Uncaught Exception { "errorType": "TypeError", "errorMessage": "The \"msecs\" argument must be of type number. Received type string ('30000')", "code": "ERR_INVALID_ARG_TYPE", "stack": [ "TypeError [ERR_INVALID_ARG_TYPE]: The \"msecs\" argument must be of type number. Received type string ('30000')", " at validateNumber (internal/validators.js:125:11)", " at getTimerDuration (internal/timers.js:381:3)", " at TLSSocket.setStreamTimeout [as setTimeout] (internal/stream_base_commons.js:250:11)", " at RedirectableRequest.destroyOnTimeout (/opt/nodejs/node_modules/follow-redirects/index.js:159:12)", " at RedirectableRequest.emit (events.js:314:20)", " at RedirectableRequest.EventEmitter.emit (domain.js:483:12)", " at ClientRequest.eventHandlers.<computed> (/opt/nodejs/node_modules/follow-redirects/index.js:14:24)", " at ClientRequest.emit (events.js:326:22)", " at ClientRequest.EventEmitter.emit (domain.js:483:12)", " at tickOnSocket (_http_client.js:709:7)" ] }

To Reproduce

var axios = require("axios")
response = axios({ url: 'https://www.google.com', method: 'GET', timeout: '30000' }).then(res => { console.log('res:', res) }).catch(err => { console.log('err:', err) })

Try the above code with follow-redirects version v1.13.3 and v1.14.0. You will find that the error is thrown when you are using v1.14.0 and the code runs fine when using v1.13.3.

Expected behavior

No error thrown by the code.

Environment

• Axios Version: 0.21.1
• Node.js Version: 12.x, 14.x
• OS: [e.g. iOS 12.1.0, OSX 10.13.4]
• Internal dependency: follow-redirects@1.14.0

Additional context/Screenshots

[DigitalBrainJS]

technically, the timeout option is defined as timeout?: number; not timeout?: number|string; so passing strings is not a documented behavior.

[kanhaiagarwal]

Yes, but if you use axios with the 2 mentioned versions of follow-redirects, you'll see that axios is accepting a string value for timeout for the earlier version, and due to some change in the later version of follow-redirects, axios is not throwing an error, but follow-redirects is throwing it. This is happening in nodejs. Either axios should check the datatype of the timeout variable before passing it to follow-redirects, or convert a string to a number before passing it to follow-redirects.

As you can see from 2 of my screenshots, axios is not causing the problem, but rather the downstream service is causing the problem.

Since I am a consumer of axios, I was able to figure this out and report appropriately as axios is a highly popular http request library used in nodejs. It's upto the axios team to take it up or not.

[piiih]

I would suggest to do a parseInt in the config.timeout when creating the httpAdater, that wouldn't be good approach once the follow-redirect breaks if it receives something that isn't a number? I'll open a PR to fix this soon :)

[DigitalBrainJS]

axios doesn't have type validators for the common options, so accepting string as timeout was an incidental behavior. As you can see, even axios v0.16.0 had the timeout option defined as a number type.
So, it looks like a feature request, not a bug. In any case, it's not a big deal to convert the timeout value to a number before passing it to the axios by multiplying the string value by 1.

[jasonsaayman]

Hi, is it possible for someone to create a pull request and associated regression and unit test for this and tag me to review?

[piiih]

I'm doing this :). I think I can finish it today

[piiih]

@jasonsaayman I can't tag you as a reviewer, I guess I don't have access :(