Tweak error messages (google#330)

Notably, this changes a call sites to use Wrap instead of Errorf for constructing errors derived from non-ok http.Response.Status.
wbxyz · Feb 10, 2025 · 4ca07a3 · 4ca07a3
1 parent 447772f
commit 4ca07a3
Show file tree

Hide file tree

Showing 18 changed files with 42 additions and 39 deletions.
diff --git a/internal/gateway/client.go b/internal/gateway/client.go
@@ -47,6 +47,6 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 		log.Println(io.ReadAll(resp.Body))
 		return nil, errors.New("gateway request rejected")
 	default:
-		return nil, errors.Errorf("Request failed: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "requesting gateway")
 	}
 }
diff --git a/internal/gitx/cache.go b/internal/gitx/cache.go
@@ -76,7 +76,7 @@ func (c Cache) GetLink(repo string, contains time.Time) (uri string, err error)
 		}
 		return
 	default:
-		err = errors.Errorf("Request failed: %s", resp.Status)
+		err = errors.Wrap(errors.New(resp.Status), "making cache request")
 		return
 	}
 }
@@ -103,7 +103,7 @@ func (c Cache) Clone(ctx context.Context, s storage.Storer, fs billy.Filesystem,
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
-		return nil, errors.Errorf("Failed to fetch cache link: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching cached repo")
 	}
 	gr, err := gzip.NewReader(resp.Body)
 	if err != nil {

diff --git a/internal/proxy/dockerfs/dockerfs.go b/internal/proxy/dockerfs/dockerfs.go
@@ -55,7 +55,7 @@ func (c Filesystem) Open(path string) (*File, error) {
 	case http.StatusBadRequest:
 		return nil, fs.ErrNotExist
 	default:
-		return nil, errors.Errorf("response error: Unexpected HTTP response: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "response error: Unexpected HTTP response")
 	}
 	tr := tar.NewReader(resp.Body)
 	hdr, err := tr.Next()
@@ -96,7 +96,7 @@ func (c Filesystem) Stat(path string) (*FileInfo, error) {
 	case http.StatusBadRequest:
 		return nil, errors.New("request error: bad parameter")
 	default:
-		return nil, errors.Errorf("response error: Unexpected HTTP response: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "response error: Unexpected HTTP response")
 	}
 	encoded := resp.Header.Get(statHeader)
 	if encoded == "" {
@@ -234,7 +234,7 @@ func (c Filesystem) WriteFile(f *File) error {
 		// TODO: Confirm the conditions under which this occurs.
 		return fs.ErrNotExist
 	default:
-		return errors.Errorf("unexpected HTTP response: %s", resp.Status)
+		return errors.Wrap(errors.New(resp.Status), "unexpected HTTP response")
 	}
 }
 

diff --git a/internal/verifier/summary.go b/internal/verifier/summary.go
@@ -46,11 +46,11 @@ func SummarizeArtifacts(ctx context.Context, metadata rebuild.LocatableAssetStor
 	req, _ := http.NewRequest(http.MethodGet, up.URI, nil)
 	resp, err := rebuild.DoContext(ctx, req)
 	if err != nil {
-		err = errors.Wrap(err, "error fetching upstream artifact")
+		err = errors.Wrap(err, "fetching upstream artifact")
 		return
 	}
 	if resp.StatusCode != 200 {
-		err = errors.Errorf("non-OK status fetching upstream artifact")
+		err = errors.Wrap(errors.New(resp.Status), "fetching upstream artifact")
 		return
 	}
 	err = archive.Stabilize(up.StabilizedHash, io.TeeReader(resp.Body, up.Hash), t.ArchiveType())

diff --git a/pkg/rebuild/cratesio/infer.go b/pkg/rebuild/cratesio/infer.go
@@ -59,10 +59,10 @@ func (Rebuilder) CloneRepo(ctx context.Context, t rebuild.Target, repoURI string
 	switch err {
 	case nil:
 	case transport.ErrAuthenticationRequired:
-		err = errors.Errorf("Repo invalid or private")
+		err = errors.Errorf("repo invalid or private [repo=%s]", r.URI)
 		return
 	default:
-		err = errors.Wrapf(err, "Clone failed [repo=%s]", r.URI)
+		err = errors.Wrapf(err, "clone failed [repo=%s]", r.URI)
 		return
 	}
 	// Do Cargo.toml search.

diff --git a/pkg/rebuild/maven/infer.go b/pkg/rebuild/maven/infer.go
@@ -41,7 +41,7 @@ func (Rebuilder) CloneRepo(ctx context.Context, t rebuild.Target, repoURI string
 	switch err {
 	case nil:
 	case transport.ErrAuthenticationRequired:
-		err = errors.Errorf("repo invalid or private")
+		err = errors.Errorf("repo invalid or private [repo=%s]", r.URI)
 		return
 	default:
 		err = errors.Wrapf(err, "clone failed [repo=%s]", r.URI)

diff --git a/pkg/rebuild/npm/infer.go b/pkg/rebuild/npm/infer.go
@@ -53,10 +53,10 @@ func (Rebuilder) CloneRepo(ctx context.Context, t rebuild.Target, repoURI string
 	switch err {
 	case nil:
 	case transport.ErrAuthenticationRequired:
-		err = errors.Errorf("Repo invalid or private")
+		err = errors.Errorf("repo invalid or private [repo=%s]", r.URI)
 		return
 	default:
-		err = errors.Wrapf(err, "Clone failed [repo=%s]", r.URI)
+		err = errors.Wrapf(err, "clone failed [repo=%s]", r.URI)
 		return
 	}
 	// Do package.json search.

diff --git a/pkg/rebuild/pypi/infer.go b/pkg/rebuild/pypi/infer.go
@@ -116,10 +116,10 @@ func (Rebuilder) CloneRepo(ctx context.Context, t rebuild.Target, repoURI string
 	switch err {
 	case nil:
 	case transport.ErrAuthenticationRequired:
-		err = errors.Errorf("repo invalid or private")
+		err = errors.Errorf("repo invalid or private [repo=%s]", r.URI)
 		return
 	default:
-		err = errors.Wrapf(err, "Clone failed [repo=%s]", r.URI)
+		err = errors.Wrapf(err, "clone failed [repo=%s]", r.URI)
 		return
 	}
 	return

diff --git a/pkg/registry/cratesio/cratesio.go b/pkg/registry/cratesio/cratesio.go
@@ -74,7 +74,7 @@ func (r HTTPRegistry) Crate(ctx context.Context, pkg string) (*Crate, error) {
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("crates.io registry error: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching crate metadata")
 	}
 	var c Crate
 	if err := json.NewDecoder(resp.Body).Decode(&c); err != nil {
@@ -102,7 +102,7 @@ func (r HTTPRegistry) Version(ctx context.Context, pkg, version string) (*CrateV
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("crates.io registry error: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching version")
 	}
 	var v CrateVersion
 	if err := json.NewDecoder(resp.Body).Decode(&v); err != nil {
@@ -128,7 +128,7 @@ func (r HTTPRegistry) Artifact(ctx context.Context, pkg string, version string)
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("fetching artifact: %s", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching artifact")
 	}
 	return resp.Body, nil
 }

diff --git a/pkg/registry/cratesio/cratesio_test.go b/pkg/registry/cratesio/cratesio_test.go
@@ -71,7 +71,7 @@ func TestHTTPRegistry_Crate(t *testing.T) {
 				URL:      "https://crates.io/api/v1/crates/nonexistent-pkg",
 				Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 			},
-			expectedErr: errors.New("crates.io registry error: Not Found"),
+			expectedErr: errors.New("fetching crate metadata: Not Found"),
 		},
 		{
 			name: "JSON Decode Error",
@@ -152,7 +152,7 @@ func TestHTTPRegistry_Version(t *testing.T) {
 			call: httpxtest.Call{URL: "https://crates.io/api/v1/crates/nonexistent-pkg/1.0.0",
 				Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 			},
-			expectedErr: errors.New("crates.io registry error: Not Found"),
+			expectedErr: errors.New("fetching version: Not Found"),
 		},
 		{
 			name:    "JSON Decode Error",
@@ -242,7 +242,7 @@ func TestHTTPRegistry_Artifact(t *testing.T) {
 					Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 				},
 			},
-			expectedErr: errors.New("crates.io registry error: Not Found"),
+			expectedErr: errors.New("fetching version: Not Found"),
 		},
 		{
 			name:    "Artifact Fetch Error",

diff --git a/pkg/registry/debian/debian.go b/pkg/registry/debian/debian.go
@@ -47,7 +47,7 @@ func (r HTTPRegistry) get(ctx context.Context, url string) (io.ReadCloser, error
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("fetching artifact: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching artifact")
 	}
 	return resp.Body, nil
 }

diff --git a/pkg/registry/maven/maven.go b/pkg/registry/maven/maven.go
@@ -110,7 +110,7 @@ func (r HTTPRegistry) PackageVersion(ctx context.Context, pkg, version string) (
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("maven registry error: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching package version")
 	}
 	defer resp.Body.Close()
 	var s search
@@ -160,7 +160,7 @@ func (r HTTPRegistry) ReleaseFile(ctx context.Context, pkg, version string, typ
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("maven registry error: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching release artifact")
 	}
 
 	return resp.Body, nil

diff --git a/pkg/registry/npm/npm.go b/pkg/registry/npm/npm.go
@@ -92,7 +92,7 @@ func (r HTTPRegistry) Package(ctx context.Context, pkg string) (*NPMPackage, err
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("npm registry error: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching package")
 	}
 	var p NPMPackage
 	if err := json.NewDecoder(resp.Body).Decode(&p); err != nil {
@@ -125,7 +125,7 @@ func (r HTTPRegistry) Version(ctx context.Context, pkg, version string) (*NPMVer
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("npm registry error: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching version")
 	}
 	var v NPMVersion
 	if err := json.NewDecoder(resp.Body).Decode(&v); err != nil {
@@ -155,7 +155,7 @@ func (r HTTPRegistry) Artifact(ctx context.Context, pkg, version string) (io.Rea
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("fetching artifact: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching artifact")
 	}
 	return resp.Body, nil
 }

diff --git a/pkg/registry/npm/npm_test.go b/pkg/registry/npm/npm_test.go
@@ -85,7 +85,7 @@ func TestHTTPRegistry_Package(t *testing.T) {
 				URL:      "https://registry.npmjs.org/invalid-package",
 				Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 			},
-			expectedErr: errors.New("npm registry error: Not Found"),
+			expectedErr: errors.New("fetching package: Not Found"),
 		},
 		{
 			name: "JSON Decode Error",
@@ -191,7 +191,7 @@ func TestHTTPRegistry_Version(t *testing.T) {
 				URL:      "https://registry.npmjs.org/invalid-package/1.0.0",
 				Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 			},
-			expectedErr: errors.New("npm registry error: Not Found"),
+			expectedErr: errors.New("fetching version: Not Found"),
 		},
 		{
 			name:    "JSON Decode Error",
@@ -284,7 +284,7 @@ func TestHTTPRegistry_Artifact(t *testing.T) {
 					Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 				},
 			},
-			expectedErr: errors.New("npm registry error: Not Found"),
+			expectedErr: errors.New("fetching version: Not Found"),
 		},
 		{
 			name:    "Artifact Fetch Error",

diff --git a/pkg/registry/pypi/pypi.go b/pkg/registry/pypi/pypi.go
@@ -84,7 +84,7 @@ func (r HTTPRegistry) Project(ctx context.Context, pkg string) (*Project, error)
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("pypi registry error: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching project")
 	}
 	var p Project
 	if err := json.NewDecoder(resp.Body).Decode(&p); err != nil {
@@ -105,7 +105,7 @@ func (r HTTPRegistry) Release(ctx context.Context, pkg, version string) (*Releas
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, errors.Errorf("pypi registry error: %v", resp.Status)
+		return nil, errors.Wrap(errors.New(resp.Status), "fetching release")
 	}
 	var release Release
 	if err := json.NewDecoder(resp.Body).Decode(&release); err != nil {
@@ -128,7 +128,7 @@ func (r HTTPRegistry) Artifact(ctx context.Context, pkg, version, filename strin
 				return nil, err
 			}
 			if resp.StatusCode != 200 {
-				return nil, errors.Errorf("fetching artifact: %v", resp.Status)
+				return nil, errors.Wrap(errors.New(resp.Status), "fetching artifact")
 			}
 			return resp.Body, nil
 		}

diff --git a/pkg/registry/pypi/pypi_test.go b/pkg/registry/pypi/pypi_test.go
@@ -71,7 +71,7 @@ func TestHTTPRegistry_Project(t *testing.T) {
 				URL:      "https://pypi.org/pypi/nonexistent-pkg/json",
 				Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 			},
-			expectedErr: errors.New("pypi registry error: Not Found"),
+			expectedErr: errors.New("fetching project: Not Found"),
 		},
 		{
 			name: "JSON Decode Error",
@@ -165,7 +165,7 @@ func TestHTTPRegistry_Release(t *testing.T) {
 				URL:      "https://pypi.org/pypi/nonexistent-pkg/1.0.0/json",
 				Response: &http.Response{StatusCode: 404, Status: http.StatusText(404)},
 			},
-			expectedErr: errors.New("pypi registry error: Not Found"),
+			expectedErr: errors.New("fetching release: Not Found"),
 		},
 		{
 			name:    "JSON Decode Error",

diff --git a/tools/benchmark/generate/main.go b/tools/benchmark/generate/main.go
@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"context"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -129,14 +130,14 @@ func get(ctx context.Context, url string) (io.ReadCloser, error) {
 	client := http.DefaultClient
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
-		return nil, fmt.Errorf("creating request: %v", err)
+		return nil, err
 	}
 	resp, err := client.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("fetching: %v", err)
+		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, fmt.Errorf("non 200 status: %s", resp.Status)
+		return nil, errors.New(resp.Status)
 	}
 	return resp.Body, nil
 }

diff --git a/tools/ctl/rundex/rundex.go b/tools/ctl/rundex/rundex.go
@@ -156,7 +156,9 @@ func cleanVerdict(m string) string {
 		m = "unsupported generator: " + m[strings.LastIndex(m, ":")+3:len(m)-2]
 	case strings.HasPrefix(m, `built version does not match requested version`):
 		m = "built version does not match requested version"
-	case strings.HasPrefix(m, "rebuild failure: Clone failed"):
+	case strings.HasPrefix(strings.ToLower(m), "rebuild failure: repo invalid or private"):
+		m = "repo invalid or private"
+	case strings.HasPrefix(strings.ToLower(m), "rebuild failure: clone failed"):
 		m = "clone failed"
 	case strings.Contains(m, "Failed to extract upstream") && strings.Contains(m, ".dist-info/WHEEL: file does not exist"):
 		m = "failed to extract upstream WHEEL"