fix: name the HTTP status in upload, evaluate and Bitbucket errors

cmd/kosli/attestationSubmit_test.go

@@ -321,7 +321,7 @@ func TestAttestationSubmitterAttestsNothingWhenTheBucketRefusesTheFile(t *testin
 	err := submitter.submit(submission)
 
 	require.EqualError(t, err, "failed to upload attachment testdata/file1: "+
-		"upload refused with HTTP 403: BadDigest: The SHA256 you specified did not match what we received")
+		"upload refused with HTTP 403 Forbidden: BadDigest: The SHA256 you specified did not match what we received")
 	require.Empty(t, fake.recorded)
 }
 

cmd/kosli/evaluateHelpers.go

@@ -202,7 +202,7 @@ func fetchRemotePolicy(remoteURL string) ([]byte, error) {
 	}
 	defer func() { _ = resp.Body.Close() }()
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return nil, fmt.Errorf("failed to fetch policy from %s: HTTP %d", remoteURL, resp.StatusCode)
+		return nil, fmt.Errorf("failed to fetch policy from %s: %s", remoteURL, requests.HTTPStatus(resp.StatusCode))
 	}
 
 	body, err := io.ReadAll(io.LimitReader(resp.Body, policyMaxBytes+1))
@@ -421,9 +421,9 @@ func serverSideRequestError(err error) error {
 	// Quoting what arrived instead would dress a proxy's page, or the decoder's
 	// complaint about one, as the server's own reason.
 	if !apiError.HasServerMessage || apiError.Message == "" {
-		return fmt.Errorf("the Kosli server answered %d", apiError.StatusCode)
+		return fmt.Errorf("the Kosli server answered %s", requests.HTTPStatus(apiError.StatusCode))
 	}
-	return fmt.Errorf("the Kosli server answered %d: %s", apiError.StatusCode, apiError.Message)
+	return fmt.Errorf("the Kosli server answered %s: %s", requests.HTTPStatus(apiError.StatusCode), apiError.Message)
 }
 
 // serverSideReadError reports a failure to read a verdict back. By this point

cmd/kosli/evaluateHelpers_test.go

@@ -3,9 +3,12 @@ package main
 import (
 	"bytes"
 	"encoding/json"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/kosli-dev/cli/internal/evaluate"
+	"github.com/kosli-dev/cli/internal/requests"
 	"github.com/stretchr/testify/require"
 )
 
@@ -21,3 +24,24 @@ func TestEvaluateResultKeysListsEveryKeyTheOutputCanHave(t *testing.T) {
 		require.Contains(t, evaluateResultKeys, key)
 	}
 }
+
+func TestFetchRemotePolicyNamesTheStatusItWasRefusedWith(t *testing.T) {
+	server := httptest.NewServer(http.NotFoundHandler())
+	defer server.Close()
+
+	_, err := fetchRemotePolicy(server.URL + "/policy.rego")
+
+	require.EqualError(t, err, "failed to fetch policy from "+server.URL+"/policy.rego: HTTP 404 Not Found")
+}
+
+func TestServerSideRequestErrorNamesTheStatus(t *testing.T) {
+	require.EqualError(t,
+		serverSideRequestError(&requests.APIError{StatusCode: http.StatusForbidden, Message: "not entitled", HasServerMessage: true}),
+		"the Kosli server answered HTTP 403 Forbidden: not entitled")
+	require.EqualError(t,
+		serverSideRequestError(&requests.APIError{StatusCode: http.StatusNotFound}),
+		"the Kosli server answered HTTP 404 Not Found")
+	require.EqualError(t,
+		serverSideRequestError(&requests.APIError{StatusCode: http.StatusBadRequest, HasServerMessage: true}),
+		"the Kosli server answered HTTP 400 Bad Request")
+}

internal/bitbucket/bitbucket.go

@@ -244,7 +244,7 @@ func (c *Config) getPullRequestDetailsFromBitbucket(prApiUrl, prHtmlLink, commit
 			}
 		}
 	} else {
-		return evidence, fmt.Errorf("failed to get PR details, got HTTP status %d. Please review repository permissions", response.Resp.StatusCode)
+		return evidence, fmt.Errorf("failed to get PR details, got %s instead of HTTP 200 OK", requests.HTTPStatus(response.Resp.StatusCode))
 	}
 	return evidence, nil
 }
@@ -332,7 +332,7 @@ func (c *Config) getPullRequestCommitsFromBitbucket(prID int) ([]types.Commit, e
 			return nil, err
 		}
 		if response.Resp.StatusCode != 200 {
-			return nil, fmt.Errorf("failed to get PR commits, got HTTP status %d", response.Resp.StatusCode)
+			return nil, fmt.Errorf("failed to get PR commits, got %s instead of HTTP 200 OK", requests.HTTPStatus(response.Resp.StatusCode))
 		}
 
 		var responseData map[string]any

internal/bitbucket/odd_replies_test.go

@@ -160,11 +160,21 @@ func (rt redirectTransport) RoundTrip(req *http.Request) (*http.Response, error)
 // runOddReplies serves replies and collects evidence for oddCommit through
 // the given entry point: 1 for PREvidenceForCommitV1, 2 for V2.
 func runOddReplies(t *testing.T, version int, replies map[string]string) ([]*types.PREvidence, error) {
+	t.Helper()
+	return runOddRepliesWithStatus(t, version, replies, nil)
+}
+
+// runOddRepliesWithStatus is runOddReplies with the status of each named reply
+// set by statuses; any other reply is a 200.
+func runOddRepliesWithStatus(t *testing.T, version int, replies map[string]string, statuses map[string]int) ([]*types.PREvidence, error) {
 	t.Helper()
 	var base string
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		for name, route := range oddRoutes {
 			if r.URL.RequestURI() == route {
+				if status, ok := statuses[name]; ok {
+					w.WriteHeader(status)
+				}
 				_, _ = w.Write([]byte(strings.ReplaceAll(replies[name], "{{base}}", base)))
 				return
 			}
@@ -423,3 +433,17 @@ func TestOddReplies_CommitWithNoHashFails(t *testing.T) {
 		})
 	}
 }
+
+// A 201 or 202 passes the client as a success but is not the reply the code
+// reads, so it fails, naming the status.
+func TestOddReplies_ASuccessOtherThanOKIsNamed(t *testing.T) {
+	for name, want := range map[string]string{
+		"pullrequest":           "failed to get PR details, got HTTP 202 Accepted instead of HTTP 200 OK",
+		"pullrequest_commits_1": "failed to get PR commits, got HTTP 202 Accepted instead of HTTP 200 OK",
+	} {
+		t.Run(name, func(t *testing.T) {
+			_, err := runOddRepliesWithStatus(t, 2, oddBaselineReplies(), map[string]int{name: http.StatusAccepted})
+			require.EqualError(t, err, want)
+		})
+	}
+}

internal/requests/requests.go

@@ -498,9 +498,9 @@ func s3ErrorMessage(status int, body []byte) string {
 		Message string `xml:"Message"`
 	}
 	if err := xml.Unmarshal(body, &s3Error); err != nil || s3Error.Code == "" {
-		return fmt.Sprintf("upload refused with HTTP %d", status)
+		return "upload refused with " + HTTPStatus(status)
 	}
-	return fmt.Sprintf("upload refused with HTTP %d: %s: %s", status, s3Error.Code, s3Error.Message)
+	return fmt.Sprintf("upload refused with %s: %s: %s", HTTPStatus(status), s3Error.Code, s3Error.Message)
 }
 
 func (c *Client) PayloadOutput(req *http.Request, jsonFields map[string]any, message string) error {
@@ -553,11 +553,11 @@ func isSuccess(status int) bool {
 func nonJSONErrorMessage(status int, body []byte, debug bool) string {
 	switch {
 	case len(bytes.TrimSpace(body)) == 0:
-		return httpStatus(status)
+		return HTTPStatus(status)
 	case debug:
-		return httpStatus(status) + ": the response is not JSON"
+		return HTTPStatus(status) + ": the response is not JSON"
 	default:
-		return httpStatus(status) + ": the response is not JSON, rerun with --debug to see it"
+		return HTTPStatus(status) + ": the response is not JSON, rerun with --debug to see it"
 	}
 }
 
@@ -569,17 +569,17 @@ func giveUpErrorHandler(resp *http.Response, err error, numTries int) (*http.Res
 	if resp != nil {
 		_, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
 		_ = resp.Body.Close()
-		msg += ": " + httpStatus(resp.StatusCode)
+		msg += ": " + HTTPStatus(resp.StatusCode)
 	}
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", msg, err)
 	}
 	return nil, errors.New(msg)
 }
 
-// httpStatus renders a code as "HTTP 409 Conflict", or "HTTP 599" for a code
+// HTTPStatus renders a code as "HTTP 409 Conflict", or "HTTP 599" for a code
 // with no standard text.
-func httpStatus(code int) string {
+func HTTPStatus(code int) string {
 	return strings.TrimSpace(fmt.Sprintf("HTTP %d %s", code, http.StatusText(code)))
 }
 

internal/requests/requests_test.go

@@ -693,8 +693,8 @@ func TestRequestsTestSuite(t *testing.T) {
 }
 
 func TestHTTPStatus(t *testing.T) {
-	require.Equal(t, "HTTP 429 Too Many Requests", httpStatus(http.StatusTooManyRequests))
-	require.Equal(t, "HTTP 599", httpStatus(599))
+	require.Equal(t, "HTTP 429 Too Many Requests", HTTPStatus(http.StatusTooManyRequests))
+	require.Equal(t, "HTTP 599", HTTPStatus(599))
 }
 
 func TestJitteredBackoff(t *testing.T) {