From 4ca07a3d6af546659a6c40052c4111c5c564a5ba Mon Sep 17 00:00:00 2001 From: Matthew Suozzo Date: Mon, 10 Feb 2025 13:52:04 -0500 Subject: [PATCH] Tweak error messages (#330) Notably, this changes a call sites to use Wrap instead of Errorf for constructing errors derived from non-ok http.Response.Status. --- internal/gateway/client.go | 2 +- internal/gitx/cache.go | 4 ++-- internal/proxy/dockerfs/dockerfs.go | 6 +++--- internal/verifier/summary.go | 4 ++-- pkg/rebuild/cratesio/infer.go | 4 ++-- pkg/rebuild/maven/infer.go | 2 +- pkg/rebuild/npm/infer.go | 4 ++-- pkg/rebuild/pypi/infer.go | 4 ++-- pkg/registry/cratesio/cratesio.go | 6 +++--- pkg/registry/cratesio/cratesio_test.go | 6 +++--- pkg/registry/debian/debian.go | 2 +- pkg/registry/maven/maven.go | 4 ++-- pkg/registry/npm/npm.go | 6 +++--- pkg/registry/npm/npm_test.go | 6 +++--- pkg/registry/pypi/pypi.go | 6 +++--- pkg/registry/pypi/pypi_test.go | 4 ++-- tools/benchmark/generate/main.go | 7 ++++--- tools/ctl/rundex/rundex.go | 4 +++- 18 files changed, 42 insertions(+), 39 deletions(-) diff --git a/internal/gateway/client.go b/internal/gateway/client.go index 3f8ebb0d..55370b87 100644 --- 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 index c8239243..3e6a65b3 100644 --- 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 index a1320b3d..2a0e24b1 100644 --- 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 index 5bbf58ae..3b920bfe 100644 --- 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 index 7b369953..c2ebddf0 100644 --- 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 index df216b6b..0d2add23 100644 --- 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 index a4fe9e9a..7fd79775 100644 --- 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 index a815a539..d63db112 100644 --- 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 index cc9b8386..fe81283c 100644 --- 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 index f916f914..344b6947 100644 --- 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 index 4c0af039..19a9f7b8 100644 --- 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 index 64bf1def..b99c1d63 100644 --- 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 index ae272292..73e34738 100644 --- 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 index 50572257..640cd87f 100644 --- 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 index 0567f49f..a1e451c5 100644 --- 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 index 26ce3810..346d4f39 100644 --- 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 index 90258779..992ecd55 100644 --- 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 index 66d3c9b4..f5e56de2 100644 --- 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"