Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Node.js v13+ default timeout changed #55

Closed
jonchurch opened this issue Feb 27, 2020 · 1 comment · Fixed by #60
Closed

Node.js v13+ default timeout changed #55

jonchurch opened this issue Feb 27, 2020 · 1 comment · Fixed by #60

Comments

@jonchurch
Copy link

jonchurch commented Feb 27, 2020

Hey there, wanted to point out that under Node.js v13+ the default timeout has changed from 2 minutes to 0. This has the effect of never timing out.

In your current implementation, under Node.js v13, your requests will never timeout on the client side.

The change would need to be made here:

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

You can hardcode the previous value of 120000 if you want to preserve the behavior of previous Node.js versions (this value has been 120000 since it was introduced to Node.js, until the recent change).

I opened #56 with the proposed change.

I've been searching Github for codebases that rely on this timeout value and manually opening issues/PRs like this. My intent is to bring your attention to this subtle change and let you decide if it's necessary to address ❤️

@epistemancer
Copy link
Contributor

@jonchurch: thanks for the heads-up! We're going to hardcode it to 30s instead of 2m -- that's a better ceiling for our API calls -- so I'll do that in a new PR instead of merging yours, but appreciate your contribution.

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