diff --git a/category_test.go b/category_test.go index 9f5b2eb..eab1b91 100644 --- a/category_test.go +++ b/category_test.go @@ -3,7 +3,6 @@ package spotify import ( "context" "net/http" - "strings" "testing" ) @@ -93,8 +92,8 @@ func TestGetCategoryPlaylistsOpt(t *testing.T) { defer server.Close() _, err := client.GetCategoryPlaylists(context.Background(), "id", Limit(5), Offset(10)) - if err == nil || !strings.Contains(err.Error(), "HTTP 404: Not Found") { - t.Errorf("Expected error 'spotify: HTTP 404: Not Found (body empty)', got %v", err) + if want := "Not Found"; err == nil || err.Error() != want { + t.Errorf("Expected error: want %v, got %v", want, err) } } diff --git a/playlist_test.go b/playlist_test.go index d09333a..13c610c 100644 --- a/playlist_test.go +++ b/playlist_test.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "net/http" - "strings" "testing" "time" ) @@ -606,7 +605,7 @@ func TestClient_ReplacePlaylistItems(t *testing.T) { items: []URI{"spotify:track:track1", "spotify:track:track2"}, }, want: want{ - err: "spotify: HTTP 403: Forbidden (body empty)", + err: "Forbidden", }, }, } @@ -714,8 +713,8 @@ func TestReorderPlaylistRequest(t *testing.T) { RangeStart: 3, InsertBefore: 8, }) - if err == nil || !strings.Contains(err.Error(), "HTTP 404: Not Found") { - t.Errorf("Expected error 'spotify: HTTP 404: Not Found (body empty)', got %v", err) + if want := "Not Found"; err == nil || err.Error() != want { + t.Errorf("Expected error: want %v, got %v", want, err) } } diff --git a/spotify.go b/spotify.go index 54a4f51..c04e5c2 100644 --- a/spotify.go +++ b/spotify.go @@ -30,10 +30,6 @@ const ( // defaultRetryDurationS helps us fix an apparent server bug whereby we will // be told to retry but not be given a wait-interval. defaultRetryDuration = time.Second * 5 - - // rateLimitExceededStatusCode is the code that the server returns when our - // request frequency is too high. - rateLimitExceededStatusCode = 429 ) // Client is a client for working with the Spotify Web API. @@ -159,11 +155,22 @@ func (e Error) Error() string { } // decodeError decodes an Error from an io.Reader. -func (c *Client) decodeError(resp *http.Response) error { +func decodeError(resp *http.Response) error { responseBody, err := io.ReadAll(resp.Body) if err != nil { return err } + if ctHeader := resp.Header.Get("Content-Type"); ctHeader == "" { + msg := string(responseBody) + if len(msg) == 0 { + msg = http.StatusText(resp.StatusCode) + } + + return Error{ + Message: msg, + Status: resp.StatusCode, + } + } if len(responseBody) == 0 { return fmt.Errorf("spotify: HTTP %d: %s (body empty)", resp.StatusCode, http.StatusText(resp.StatusCode)) @@ -239,7 +246,7 @@ func (c *Client) execute(req *http.Request, result interface{}, needsStatus ...i if (resp.StatusCode >= 300 || resp.StatusCode < 200) && isFailure(resp.StatusCode, needsStatus) { - return c.decodeError(resp) + return decodeError(resp) } if result != nil { @@ -280,7 +287,7 @@ func (c *Client) get(ctx context.Context, url string, result interface{}) error defer resp.Body.Close() - if resp.StatusCode == rateLimitExceededStatusCode && c.autoRetry { + if resp.StatusCode == http.StatusTooManyRequests && c.autoRetry { select { case <-ctx.Done(): // If the context is cancelled, return the original error @@ -292,7 +299,7 @@ func (c *Client) get(ctx context.Context, url string, result interface{}) error return nil } if resp.StatusCode != http.StatusOK { - return c.decodeError(resp) + return decodeError(resp) } return json.NewDecoder(resp.Body).Decode(result) diff --git a/spotify_test.go b/spotify_test.go index 9eb11e0..635e2ac 100644 --- a/spotify_test.go +++ b/spotify_test.go @@ -2,7 +2,6 @@ package spotify import ( "context" - "golang.org/x/oauth2" "io" "net/http" "net/http/httptest" @@ -10,6 +9,8 @@ import ( "strings" "testing" "time" + + "golang.org/x/oauth2" ) func testClient(code int, body io.Reader, validators ...func(*http.Request)) (*Client, *httptest.Server) { @@ -70,7 +71,7 @@ func TestNewReleasesRateLimitExceeded(t *testing.T) { // first attempt fails http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Retry-After", "2") - w.WriteHeader(rateLimitExceededStatusCode) + w.WriteHeader(http.StatusTooManyRequests) _, _ = io.WriteString(w, `{ "error": { "message": "slow down", "status": 429 } }`) }), // next attempt succeeds @@ -144,14 +145,14 @@ func TestClient_Token(t *testing.T) { t.Run("non oauth2 transport", func(t *testing.T) { client := &Client{ - http: http.DefaultClient, + http: http.DefaultClient, } _, err := client.Token() if err == nil || err.Error() != "spotify: client not backed by oauth2 transport" { t.Errorf("Should throw error: %s", "spotify: client not backed by oauth2 transport") } }) - + t.Run("invalid token", func(t *testing.T) { httpClient := config.Client(context.Background(), nil) client := New(httpClient) @@ -161,3 +162,19 @@ func TestClient_Token(t *testing.T) { } }) } + +func TestDecode429Error(t *testing.T) { + resp := &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: http.Header{"Retry-After": []string{"2"}}, + Body: io.NopCloser(strings.NewReader(`Too many requests`)), + } + + err := decodeError(resp) + if err == nil { + t.Fatal("Expected error") + } + if err.Error() != "Too many requests" { + t.Error("Invalid error message:", err.Error()) + } +} diff --git a/user_test.go b/user_test.go index 5132585..c68d9f1 100644 --- a/user_test.go +++ b/user_test.go @@ -129,7 +129,7 @@ func TestFollowArtistAutoRetry(t *testing.T) { // first attempt fails http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Retry-After", "2") - w.WriteHeader(rateLimitExceededStatusCode) + w.WriteHeader(http.StatusTooManyRequests) _, _ = io.WriteString(w, `{ "error": { "message": "slow down", "status": 429 } }`) }), // next attempt succeeds