commit 633fee62fe8f6354c37d1f13861cb76ca0db4bbb
author Kevin Lubick <kjlubick@google.com> Mon Sep 18 14:11:12 2023 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Mon Sep 18 18:29:22 2023 +0000
tree ff4780bd1ee89f290b77b7d8f3f50edb4417c154
parent aea31dd66eb1e4a4cb21d12e212fc9fdcb1bf5b6

Use skerr.Wrap more in httputils

We got a bit confused when diagnosing a failing RPC call because
we weren't sure where the "return error on non 2XX" logic was
coming in. Had we been wrapping the error, the file location
would have been more clear.

Change-Id: I81d2a7d908b2633ad0f1f474cd847d787109f7d0
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/757316
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>

go/httputils/BUILD.bazel

diff --git a/go/httputils/BUILD.bazel b/go/httputils/BUILD.bazel
index 08a3356..009b937 100644
--- a/go/httputils/BUILD.bazel
+++ b/go/httputils/BUILD.bazel
@@ -8,6 +8,7 @@
     visibility = ["//visibility:public"],
     deps = [
         "//go/metrics2",
+        "//go/skerr",
         "//go/sklog",
         "//go/timer",
         "//go/util",

go/httputils/http.go

diff --git a/go/httputils/http.go b/go/httputils/http.go
index 7b457d3..9d36ad9 100644
--- a/go/httputils/http.go
+++ b/go/httputils/http.go
@@ -6,7 +6,6 @@
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net"
 	"net/http"
 	"net/url"
@@ -20,6 +19,7 @@
 	"golang.org/x/oauth2"
 
 	"go.skia.org/infra/go/metrics2"
+	"go.skia.org/infra/go/skerr"
 	"go.skia.org/infra/go/sklog"
 	"go.skia.org/infra/go/timer"
 	"go.skia.org/infra/go/util"
@@ -243,7 +243,7 @@
 func (t Response2xxOnlyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	resp, err := t.RoundTripper.RoundTrip(req)
 	if err == nil && resp != nil && (resp.StatusCode < 200 || resp.StatusCode > 299) {
-		return nil, fmt.Errorf("Got error response status code %d from the HTTP %s request to %s\nResponse: %s", resp.StatusCode, req.Method, req.URL, ReadAndClose(resp.Body))
+		return nil, skerr.Fmt("Got error response status code %d from the HTTP %s request to %s\nResponse: %s", resp.StatusCode, req.Method, req.URL, ReadAndClose(resp.Body))
 	}
 	return resp, err
 }
@@ -271,7 +271,7 @@
 func (t Response2xxAnd3xxTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	resp, err := t.RoundTripper.RoundTrip(req)
 	if err == nil && resp != nil && (resp.StatusCode < 200 || resp.StatusCode > 399) {
-		return nil, fmt.Errorf("Got error response status code %d from the HTTP %s request to %s\nResponse: %s", resp.StatusCode, req.Method, req.URL, ReadAndClose(resp.Body))
+		return nil, skerr.Fmt("Got error response status code %d from the HTTP %s request to %s\nResponse: %s", resp.StatusCode, req.Method, req.URL, ReadAndClose(resp.Body))
 	}
 	return resp, err
 }
@@ -359,7 +359,7 @@
 	bodyBuf := bytes.Buffer{}
 	if req.Body != nil {
 		if _, err := bodyBuf.ReadFrom(req.Body); err != nil {
-			return nil, fmt.Errorf("Failed to read request body: %v", err)
+			return nil, skerr.Wrapf(err, "Failed to read request body")
 		}
 	}
 
@@ -367,7 +367,7 @@
 	var err error
 	roundTripOp := func() error {
 		if req.Body != nil {
-			req.Body = ioutil.NopCloser(bytes.NewBufferString(bodyBuf.String()))
+			req.Body = io.NopCloser(bytes.NewBufferString(bodyBuf.String()))
 		}
 		if resp != nil {
 			panic("Expected notifyFunc to be called between retries.")
@@ -425,7 +425,7 @@
 func ReadAndClose(r io.ReadCloser) string {
 	if r != nil {
 		defer util.Close(r)
-		if b, err := ioutil.ReadAll(io.LimitReader(r, MAX_BYTES_IN_RESPONSE_BODY)); err != nil {
+		if b, err := io.ReadAll(io.LimitReader(r, MAX_BYTES_IN_RESPONSE_BODY)); err != nil {
 			sklog.Warningf("There was a potential problem reading the response body: %s", err)
 		} else {
 			return fmt.Sprintf("%q", string(b))
@@ -557,7 +557,7 @@
 	} else {
 		val, err = strconv.Atoi(valStr)
 		if err != nil {
-			return 0, fmt.Errorf("Not a valid integer value.")
+			return 0, skerr.Wrapf(err, "Not a valid integer value %q.", valStr)
 		}
 	}
 	if val < 0 {