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

[BUG] 30s default timeout is too small #26

Closed
isaacs opened this issue Apr 22, 2020 · 2 comments
Closed

[BUG] 30s default timeout is too small #26

isaacs opened this issue Apr 22, 2020 · 2 comments

Comments

@isaacs
Copy link
Contributor

isaacs commented Apr 22, 2020

What / Why

See npm/cli#1151

The documented default timeout of 30s was not being set in v4, so we "fixed the glitch" in nrf 4.0.3, causing problems for people.

There's no npm v6 way to specify what timeout to use, so not having a timeout at all seems like a reasonable default for the v4 line, at least. Let's roll back that change, and document it. (Arguably, fixing this bug was a breaking change, and we ought to roll it back.)

For latest/npm v7, it seems like 30s is way too small. We should add a --fetch-timeout config on the cli, which defaults to 3000 (5m) or so, and also use that as the default here.

isaacs added a commit that referenced this issue Apr 23, 2020
Re: #26
Re: npm/cli#1151

The documented default timeout of 30s was not being set in v4, so we
"fixed the glitch" in v4.0.3, causing problems for people trying to
download large packages.

There's no npm v6 way to specify what timeout to use, so not having a
timeout at all seems like a reasonable default for the v4 line, at
least. Let's roll back that change, and document it. (Arguably, fixing
this bug was a breaking change, and we ought to roll it back.)

This effectively reverts 69c2977, with
documentation of the effective behavior before the change.
isaacs added a commit that referenced this issue Apr 23, 2020
Re: #26
Re: npm/cli#1151

The previous default of 30s was too small for lots of users, causing
problems when they attempt to download large objects from the npm
registry.

Bump up the default timeout to 5m.

TODO: add a `--fetch-timeout` option on the CLI to explicitly set
`timeout` in the npm.flatOptions object passed to all dependencies.
@ruyadorno
Copy link
Contributor

sounds good to me 👍

isaacs added a commit that referenced this issue Apr 28, 2020
Re: #26
Re: npm/cli#1151

The documented default timeout of 30s was not being set in v4, so we
"fixed the glitch" in v4.0.3, causing problems for people trying to
download large packages.

There's no npm v6 way to specify what timeout to use, so not having a
timeout at all seems like a reasonable default for the v4 line, at
least. Let's roll back that change, and document it. (Arguably, fixing
this bug was a breaking change, and we ought to roll it back.)

This effectively reverts 69c2977, with
documentation of the effective behavior before the change.

PR-URL: #27
Credit: @isaacs
Close: #27
Reviewed-by: @isaacs
@isaacs
Copy link
Contributor Author

isaacs commented Apr 28, 2020

Fixed by #27

@isaacs isaacs closed this as completed Apr 28, 2020
isaacs added a commit that referenced this issue Apr 28, 2020
Re: #26
Re: npm/cli#1151

The previous default of 30s was too small for lots of users, causing
problems when they attempt to download large objects from the npm
registry.

Bump up the default timeout to 5m.

TODO: add a `--fetch-timeout` option on the CLI to explicitly set
`timeout` in the npm.flatOptions object passed to all dependencies.

PR-URL: #28
Credit: @isaacs
Close: #28
Reviewed-by: @isaacs
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

No branches or pull requests

2 participants