Merge pull request #284 from flimzy/errorDetail

More consistently include the HTTP response code in errors

Author: strideynet

category_test.go

@@ -92,7 +92,7 @@ func TestGetCategoryPlaylistsOpt(t *testing.T) {
 	defer server.Close()
 
 	_, err := client.GetCategoryPlaylists(context.Background(), "id", Limit(5), Offset(10))
-	if want := "Not Found"; err == nil || err.Error() != want {
+	if want := "spotify: Not Found [404]"; err == nil || err.Error() != want {
 		t.Errorf("Expected error: want %v, got %v", want, err)
 	}
 }

playlist_test.go

@@ -605,7 +605,7 @@ func TestClient_ReplacePlaylistItems(t *testing.T) {
 				items:      []URI{"spotify:track:track1", "spotify:track:track2"},
 			},
 			want: want{
-				err: "Forbidden",
+				err: "spotify: Forbidden [403]",
 			},
 		},
 	}
@@ -713,7 +713,7 @@ func TestReorderPlaylistRequest(t *testing.T) {
 		RangeStart:   3,
 		InsertBefore: 8,
 	})
-	if want := "Not Found"; err == nil || err.Error() != want {
+	if want := "spotify: Not Found [404]"; err == nil || err.Error() != want {
 		t.Errorf("Expected error: want %v, got %v", want, err)
 	}
 }

spotify.go

@@ -154,7 +154,13 @@ type Error struct {
 }
 
 func (e Error) Error() string {
-	return e.Message
+	return fmt.Sprintf("spotify: %s [%d]", e.Message, e.Status)
+}
+
+// HTTPStatus returns the HTTP status code returned by the server when the error
+// occurred.
+func (e Error) HTTPStatus() int {
+	return e.Status
 }
 
 // decodeError decodes an Error from an io.Reader.
@@ -176,7 +182,10 @@ func decodeError(resp *http.Response) error {
 	}
 
 	if len(responseBody) == 0 {
-		return fmt.Errorf("spotify: HTTP %d: %s (body empty)", resp.StatusCode, http.StatusText(resp.StatusCode))
+		return Error{
+			Message: "server response without body",
+			Status:  resp.StatusCode,
+		}
 	}
 
 	buf := bytes.NewBuffer(responseBody)
@@ -186,18 +195,20 @@ func decodeError(resp *http.Response) error {
 	}
 	err = json.NewDecoder(buf).Decode(&e)
 	if err != nil {
-		return fmt.Errorf("spotify: couldn't decode error: (%d) [%s]", len(responseBody), responseBody)
+		return Error{
+			Message: fmt.Sprintf("failed to decode error response %q", responseBody),
+			Status:  resp.StatusCode,
+		}
 	}
 
+	e.E.Status = resp.StatusCode
 	if e.E.Message == "" {
 		// Some errors will result in there being a useful status-code but an
-		// empty message, which will confuse the user (who only has access to
-		// the message and not the code). An example of this is when we send
-		// some of the arguments directly in the HTTP query and the URL ends-up
-		// being too long.
+		// empty message. An example of this is when we send some of the
+		// arguments directly in the HTTP query and the URL ends-up being too
+		// long.
 
-		e.E.Message = fmt.Sprintf("spotify: unexpected HTTP %d: %s (empty error)",
-			resp.StatusCode, http.StatusText(resp.StatusCode))
+		e.E.Message = "server response without error description"
 	}
 	if retryAfter, _ := strconv.Atoi(resp.Header.Get("Retry-After")); retryAfter != 0 {
 		e.E.RetryAfter = time.Now().Add(time.Duration(retryAfter) * time.Second)

spotify_test.go

@@ -115,7 +115,7 @@ func TestRateLimitExceededReportsRetryAfter(t *testing.T) {
 		// first attempt fails
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			w.Header().Set("Retry-After", strconv.Itoa(retryAfter))
-			w.WriteHeader(rateLimitExceededStatusCode)
+			w.WriteHeader(http.StatusTooManyRequests)
 			_, _ = io.WriteString(w, `{ "error": { "message": "slow down", "status": 429 } }`)
 		}),
 		// next attempt succeeds
@@ -222,7 +222,16 @@ func TestDecode429Error(t *testing.T) {
 	if err == nil {
 		t.Fatal("Expected error")
 	}
-	if err.Error() != "Too many requests" {
-		t.Error("Invalid error message:", err.Error())
+	if err.Error() != "spotify: Too many requests [429]" {
+		t.Error("Unexpected error message:", err.Error())
+	}
+	const wantSTatus = http.StatusTooManyRequests
+	var gotStatus int
+	var statusErr interface{ HTTPStatus() int }
+	if errors.As(err, &statusErr) {
+		gotStatus = statusErr.HTTPStatus()
+	}
+	if gotStatus != wantSTatus {
+		t.Errorf("Expected status %d, got %d", wantSTatus, gotStatus)
 	}
 }