Skip to content

Commit 66c9ed4

Browse files
fix(install): surface real mirror download failures
Stop remapping every failed archive/checksum attempt to ""not found"". Keep the detailed error for total mirror failure; log light skips when a later mirror succeeds. Refs #1401 Co-authored-by: Cursor
1 parent 0878ab4 commit 66c9ed4

3 files changed

Lines changed: 125 additions & 3 deletions

File tree

‎src/installer/download_error.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package installer
2+
3+
import (
4+
nvmhttp "common/http"
5+
"fmt"
6+
"net/http"
7+
"strings"
8+
)
9+
10+
func describeDownloadResultFailure(label, url string, result nvmhttp.DownloadResult) error {
11+
if result.Error != nil {
12+
return fmt.Errorf("%s %s: %w", label, url, result.Error)
13+
}
14+
if result.Response == nil || result.Response.Response == nil {
15+
return fmt.Errorf("%s %s: empty response", label, url)
16+
}
17+
status := result.Response.Response.StatusCode
18+
statusText := strings.TrimSpace(result.Response.Response.Status)
19+
if statusText == "" {
20+
statusText = http.StatusText(status)
21+
}
22+
if status == http.StatusNotFound {
23+
return fmt.Errorf("%s %s: HTTP %s (missing on this mirror)", label, url, statusText)
24+
}
25+
return fmt.Errorf("%s %s: HTTP %s", label, url, statusText)
26+
}
27+
28+
func formatNodeMirrorDownloadFailure(version, archiveName string, mirrors []string, lastErr error) error {
29+
mirrorNote := "configured mirror"
30+
if len(mirrors) > 1 {
31+
mirrorNote = fmt.Sprintf("%d configured mirrors", len(mirrors))
32+
}
33+
if lastErr == nil {
34+
return fmt.Errorf(
35+
"failed to download Node.js v%s (%s) from %s",
36+
version,
37+
archiveName,
38+
mirrorNote,
39+
)
40+
}
41+
return fmt.Errorf(
42+
"failed to download Node.js v%s (%s) from %s: %w",
43+
version,
44+
archiveName,
45+
mirrorNote,
46+
lastErr,
47+
)
48+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package installer
2+
3+
import (
4+
nvmhttp "common/http"
5+
"errors"
6+
"net/http"
7+
"strings"
8+
"testing"
9+
)
10+
11+
func TestDescribeDownloadResultFailureTransport(t *testing.T) {
12+
err := describeDownloadResultFailure("archive", "https://example/a.7z", nvmhttp.DownloadResult{
13+
Error: errors.New("context deadline exceeded"),
14+
})
15+
if err == nil || !strings.Contains(err.Error(), "context deadline exceeded") {
16+
t.Fatalf("error = %v", err)
17+
}
18+
if !strings.Contains(err.Error(), "archive https://example/a.7z") {
19+
t.Fatalf("error = %v, want label+url", err)
20+
}
21+
}
22+
23+
func TestDescribeDownloadResultFailureHTTPStatus(t *testing.T) {
24+
res := &http.Response{StatusCode: http.StatusNotFound, Status: "404 Not Found"}
25+
err := describeDownloadResultFailure("checksum", "https://example/SHASUMS256.txt", nvmhttp.DownloadResult{
26+
Response: &nvmhttp.DownloadResponse{Response: res},
27+
})
28+
if err == nil || !strings.Contains(err.Error(), "HTTP 404 Not Found") {
29+
t.Fatalf("error = %v", err)
30+
}
31+
if !strings.Contains(err.Error(), "missing on this mirror") {
32+
t.Fatalf("error = %v, want missing hint", err)
33+
}
34+
}
35+
36+
func TestFormatNodeMirrorDownloadFailureWrapsCause(t *testing.T) {
37+
err := formatNodeMirrorDownloadFailure("24.20.0", "node-v24.20.0-win-x64.7z", []string{"https://nodejs.org/dist"}, errors.New("timeout"))
38+
got := err.Error()
39+
for _, want := range []string{
40+
"failed to download Node.js v24.20.0",
41+
"node-v24.20.0-win-x64.7z",
42+
"configured mirror",
43+
"timeout",
44+
} {
45+
if !strings.Contains(got, want) {
46+
t.Fatalf("error = %q, want substring %q", got, want)
47+
}
48+
}
49+
if strings.Contains(got, "not found on server/mirror") {
50+
t.Fatalf("legacy misleading message still present: %q", got)
51+
}
52+
}
53+
54+
func TestFormatNodeMirrorDownloadFailurePluralMirrors(t *testing.T) {
55+
err := formatNodeMirrorDownloadFailure("22.0.0", "node-v22.0.0-win-x64.7z", []string{"a", "b"}, nil)
56+
if !strings.Contains(err.Error(), "2 configured mirrors") {
57+
t.Fatalf("error = %q", err)
58+
}
59+
}

‎src/installer/installer.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
402402
if !fromCache && !cfg.LocalOnly {
403403
downloaded := false
404404
var downloadEnd time.Time
405+
var lastDownloadErr error
405406
insecure := allowInsecureDownloads(cfg)
406407
mirrors := settings.Global().NodeMirror
407408
singleMirror := len(mirrors) == 1
@@ -422,6 +423,8 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
422423
shasumURI := fmt.Sprintf("%s/v%s/SHASUMS256.txt", mirror, version)
423424
shasumJob, err := http.Download(shasumURI, http.DownloadConfig{Cache: true, Destination: shasumPath, AllowInsecure: insecure})
424425
if err != nil {
426+
lastDownloadErr = fmt.Errorf("checksum %s: %w", shasumURI, err)
427+
log.Logf("mirror %s skipped for v%s: %v", mirror, version, lastDownloadErr)
425428
continue
426429
}
427430

@@ -438,6 +441,12 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
438441
return authErr
439442
}
440443
}
444+
if ok {
445+
lastDownloadErr = describeDownloadResultFailure("checksum", shasumURI, result)
446+
} else {
447+
lastDownloadErr = fmt.Errorf("checksum %s: download closed unexpectedly", shasumURI)
448+
}
449+
log.Logf("mirror %s skipped for v%s: %v", mirror, version, lastDownloadErr)
441450
continue
442451
}
443452
}
@@ -446,12 +455,16 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
446455
normalizedURI, err := http.NormalizeURL(uri)
447456
if err != nil {
448457
_ = os.Remove(shasumPath)
458+
lastDownloadErr = fmt.Errorf("archive %s: %w", uri, err)
459+
log.Logf("mirror %s skipped for v%s: %v", mirror, version, lastDownloadErr)
449460
continue
450461
}
451462

452463
job, err := http.Download(normalizedURI, http.DownloadConfig{Destination: target, AllowInsecure: insecure})
453464
if err != nil {
454465
_ = os.Remove(shasumPath)
466+
lastDownloadErr = fmt.Errorf("archive %s: %w", normalizedURI, err)
467+
log.Logf("mirror %s skipped for v%s: %v", mirror, version, lastDownloadErr)
455468
continue
456469
}
457470

@@ -470,7 +483,7 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
470483
}
471484
case result, ok := <-job.Result:
472485
if !ok {
473-
downloadErr = fmt.Errorf("download closed unexpectedly")
486+
downloadErr = fmt.Errorf("archive %s: download closed unexpectedly", normalizedURI)
474487
break downloadLoop
475488
}
476489
if result.Error != nil || result.Response == nil || !result.Response.Success {
@@ -480,7 +493,7 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
480493
break downloadLoop
481494
}
482495
}
483-
downloadErr = fmt.Errorf("download error")
496+
downloadErr = describeDownloadResultFailure("archive", normalizedURI, result)
484497
break downloadLoop
485498
}
486499
downloadEnd = time.Now()
@@ -498,6 +511,8 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
498511
status.Downloads--
499512
return downloadErr
500513
}
514+
lastDownloadErr = downloadErr
515+
log.Logf("mirror %s skipped for v%s: %v", mirror, version, lastDownloadErr)
501516
continue
502517
}
503518

@@ -521,7 +536,7 @@ func downloadNode(ctx context.Context, version, target string, cfg InstallConfig
521536

522537
if !downloaded {
523538
status.Downloads--
524-
return fmt.Errorf("Node.js v%s not found on server/mirror", version)
539+
return formatNodeMirrorDownloadFailure(version, archiveName, mirrors, lastDownloadErr)
525540
}
526541

527542
if logf != nil {

0 commit comments

Comments
 (0)