Describe the bug
internal/node/dist.go:109 fetches the nodejs.org manifest via http.Get(url), which uses http.DefaultClient — no timeout is set anywhere in the request path.
Failure scenario
If nodejs.org hangs, or a corporate proxy/firewall silently drops the connection (no RST), nodeup check/nodeup upgrade blocks forever with no way out except Ctrl-C. There's no automatic fallback to the offline cache when a request merely hangs (as opposed to erroring outright).
Related, same file
- No retry/backoff on a failed fetch (a single transient 503/DNS blip fails the command outright, despite this apparently being part of the original design intent).
- No
context.Context threaded into the HTTP call (no http.NewRequestWithContext) — a live cmd.Context() from the CLI layer can't cancel an in-flight fetch even when the caller has one available.
saveToCache (dist.go:267-284) writes the cache data file and its .meta expiry file as two separate plain os.WriteFile calls with no atomicity — two concurrent nodeup invocations refreshing the cache simultaneously can produce a corrupted or mismatched cache.
Suggested fix
- Use an
http.Client with a sane timeout (e.g. 10-30s).
- Thread
ctx through via http.NewRequestWithContext and client.Do(req).
- Add a small retry-with-backoff wrapper (2-3 attempts) around the fetch.
- Make
saveToCache atomic: write to a temp file in the same directory, then os.Rename over the target (matching the pattern already used correctly in internal/packages/sentinel.go).
Severity
High for the timeout (can hang forever with no recovery), Medium for the retry/context/atomicity items.
Describe the bug
internal/node/dist.go:109fetches the nodejs.org manifest viahttp.Get(url), which useshttp.DefaultClient— no timeout is set anywhere in the request path.Failure scenario
If nodejs.org hangs, or a corporate proxy/firewall silently drops the connection (no RST),
nodeup check/nodeup upgradeblocks forever with no way out except Ctrl-C. There's no automatic fallback to the offline cache when a request merely hangs (as opposed to erroring outright).Related, same file
context.Contextthreaded into the HTTP call (nohttp.NewRequestWithContext) — a livecmd.Context()from the CLI layer can't cancel an in-flight fetch even when the caller has one available.saveToCache(dist.go:267-284) writes the cache data file and its.metaexpiry file as two separate plainos.WriteFilecalls with no atomicity — two concurrentnodeupinvocations refreshing the cache simultaneously can produce a corrupted or mismatched cache.Suggested fix
http.Clientwith a sane timeout (e.g. 10-30s).ctxthrough viahttp.NewRequestWithContextandclient.Do(req).saveToCacheatomic: write to a temp file in the same directory, thenos.Renameover the target (matching the pattern already used correctly ininternal/packages/sentinel.go).Severity
High for the timeout (can hang forever with no recovery), Medium for the retry/context/atomicity items.