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

Default http timeout changed in Node 13 #806

Closed
jonchurch opened this issue Feb 14, 2020 · 2 comments · Fixed by #807
Closed

Default http timeout changed in Node 13 #806

jonchurch opened this issue Feb 14, 2020 · 2 comments · Fixed by #807

Comments

@jonchurch
Copy link
Contributor

jonchurch commented Feb 14, 2020

In Node.js v13.0.0 the default socket timeout has been set to 0.

Currently in this Library the default timeout is being set based on require('http').createServer().timeout.

const DEFAULT_TIMEOUT = require('http').createServer().timeout;

The default previously was 120 seconds, under v13 it is no timeout by default aka a value of 0.

I noticed this when looking into #796

I haven't done any testing with this so I don't know if it represents an actual bug, but wanted to file this issue in case this was something the team was unaware of.

This timeout was introduced in Node.js v0.9.12 as 120 seconds, and did not change until v13.0.0 when it was switched to 0. There is of course a possibility that this change will be reverted before v14 is released, but it's worth keeping an eye on.

@ob-stripe
Copy link
Contributor

Thanks for the heads up @jonchurch!

@brandur-stripe @remi-stripe @richardm-stripe I think we should just change the default timeout to a hardcoded value of 80 seconds like we do in other client libraries. Does that sound good to everyone?

@brandur-stripe
Copy link
Contributor

Does that sound good to everyone?

Strong +1 from me.

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

Successfully merging a pull request may close this issue.

3 participants