fix: report non-JSON error responses by their HTTP status

internal/requests/requests.go

@@ -388,10 +388,7 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) {
 			err := json.Unmarshal([]byte(body), &respBody)
 			if err != nil {
 				c.Logger.Debug("response body from %s (status %d):\n%s", req.URL, resp.StatusCode, string(body))
-				// Still carry the status code so callers can distinguish e.g. a 404
-				// with an empty or non-JSON body (a proxy/CDN page). Keep the
-				// (JSON parse) error text as the message.
-				return nil, &APIError{StatusCode: resp.StatusCode, Message: err.Error()}
+				return nil, &APIError{StatusCode: resp.StatusCode, Message: nonJSONErrorMessage(resp.StatusCode, body, c.Debug)}
 			}
 			cleanedErrorMessage := ""
 			hasServerMessage := false
@@ -550,6 +547,20 @@ func isSuccess(status int) bool {
 	return false
 }
 
+// nonJSONErrorMessage reports an error response whose body is not JSON, such as
+// a proxy's HTML page, by its status: the decoder's complaint about the body
+// tells the user nothing.
+func nonJSONErrorMessage(status int, body []byte, debug bool) string {
+	switch {
+	case len(bytes.TrimSpace(body)) == 0:
+		return httpStatus(status)
+	case debug:
+		return httpStatus(status) + ": the response is not JSON"
+	default:
+		return httpStatus(status) + ": the response is not JSON, rerun with --debug to see it"
+	}
+}
+
 // giveUpErrorHandler keeps the last response's status, which retryablehttp's
 // default handler drops. The *url.Error that http.Client wraps it in names the
 // method and URL.

internal/requests/requests_test.go

@@ -79,6 +79,10 @@ func (suite *RequestsTestSuite) SetupSuite() {
 		Get("/fail/").
 		Reply(500).
 		BodyString("server broken")
+	suite.fakeService.NewHandler().
+		Get("/blank/").
+		Reply(401).
+		BodyString("\r\n")
 	suite.fakeService.NewHandler().
 		Get("/proxy-block/").
 		Reply(403).
@@ -323,7 +327,7 @@ func (suite *RequestsTestSuite) TestDo() {
 				URL:    suite.fakeService.ResolveURL("/no-content/"),
 			},
 			wantError:        true,
-			expectedErrorMsg: "unexpected end of JSON input",
+			expectedErrorMsg: "HTTP 204 No Content",
 		},
 		{
 			name: "206 is not a success: a partial body is not the answer",
@@ -332,7 +336,7 @@ func (suite *RequestsTestSuite) TestDo() {
 				URL:    suite.fakeService.ResolveURL("/partial/"),
 			},
 			wantError:        true,
-			expectedErrorMsg: "unexpected end of JSON input",
+			expectedErrorMsg: "HTTP 206 Partial Content: the response is not JSON, rerun with --debug to see it",
 		},
 		{
 			name: "GET request to 404 endpoint",
@@ -430,13 +434,22 @@ func (suite *RequestsTestSuite) TestDo() {
 			expectedErrorMsg: "Denied",
 		},
 		{
-			name: "GET request to a PUT endpoint fails because of invalid response",
+			name: "GET to an unrouted path reports the status of the empty 404",
 			params: &RequestParams{
 				Method: http.MethodGet,
 				URL:    suite.fakeService.ResolveURL("/html"),
 			},
 			wantError:        true,
-			expectedErrorMsg: "unexpected end of JSON input",
+			expectedErrorMsg: "HTTP 404 Not Found",
+		},
+		{
+			name: "a whitespace-only body reports the status alone",
+			params: &RequestParams{
+				Method: http.MethodGet,
+				URL:    suite.fakeService.ResolveURL("/blank/"),
+			},
+			wantError:        true,
+			expectedErrorMsg: "HTTP 401 Unauthorized",
 		},
 		{
 			name: "non-JSON error response body is logged with debug enabled",
@@ -446,7 +459,7 @@ func (suite *RequestsTestSuite) TestDo() {
 			},
 			debug:            true,
 			wantError:        true,
-			expectedErrorMsg: "invalid character '<' looking for beginning of value",
+			expectedErrorMsg: "HTTP 403 Forbidden: the response is not JSON",
 			expectedLog:      `<!DOCTYPE html><html><body><h1>403 Forbidden</h1><p>Blocked by proxy</p></body></html>`,
 		},
 	} {