Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added maxRetryDuration option to client #273

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions spotify.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,19 @@ const (
rateLimitExceededStatusCode = 429
)

// ErrNoMorePages is the error returned when response header has a 'Retry-After'
// duration longer then the configuerd max.
Pineapple217 marked this conversation as resolved.
Show resolved Hide resolved
var ErrMaxRetryDurationExceeded = errors.New("spotify: retry would take longer then configured max")
Pineapple217 marked this conversation as resolved.
Show resolved Hide resolved

// Client is a client for working with the Spotify Web API.
// It is best to create this using spotify.New()
type Client struct {
http *http.Client
baseURL string

autoRetry bool
acceptLanguage string
autoRetry bool
acceptLanguage string
maxRetryDuration time.Duration
}

type ClientOption func(client *Client)
Expand All @@ -74,6 +79,15 @@ func WithAcceptLanguage(lang string) ClientOption {
}
}

// WithMaxRetryDuration limits the amount of time that the client wil wait to retry afther being rate limited.
Pineapple217 marked this conversation as resolved.
Show resolved Hide resolved
// If the retry time is longer then the max, then the client will return an err
Pineapple217 marked this conversation as resolved.
Show resolved Hide resolved
// This option only works when auto retry is enabled
func WithMaxRetryDuration(duration time.Duration) ClientOption {
return func(client *Client) {
client.maxRetryDuration = duration
}
}

// New returns a client for working with the Spotify Web API.
// The provided httpClient must provide Authentication with the requests.
// The auth package may be used to generate a suitable client.
Expand Down Expand Up @@ -230,10 +244,14 @@ func (c *Client) execute(req *http.Request, result interface{}, needsStatus ...i
if c.autoRetry &&
isFailure(resp.StatusCode, needsStatus) &&
shouldRetry(resp.StatusCode) {
duration := retryDuration(resp)
if c.maxRetryDuration > 0 && duration > c.maxRetryDuration {
return ErrMaxRetryDurationExceeded
}
select {
case <-req.Context().Done():
// If the context is cancelled, return the original error
case <-time.After(retryDuration(resp)):
case <-time.After(duration):
continue
}
}
Expand Down Expand Up @@ -285,10 +303,14 @@ func (c *Client) get(ctx context.Context, url string, result interface{}) error
defer resp.Body.Close()

if resp.StatusCode == rateLimitExceededStatusCode && c.autoRetry {
duration := retryDuration(resp)
if c.maxRetryDuration > 0 && duration > c.maxRetryDuration {
return ErrMaxRetryDurationExceeded
}
select {
case <-ctx.Done():
// If the context is cancelled, return the original error
case <-time.After(retryDuration(resp)):
case <-time.After(duration):
continue
}
}
Expand Down
30 changes: 27 additions & 3 deletions spotify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ package spotify

import (
"context"
"golang.org/x/oauth2"
"io"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"time"

"golang.org/x/oauth2"
)

func testClient(code int, body io.Reader, validators ...func(*http.Request)) (*Client, *httptest.Server) {
Expand Down Expand Up @@ -104,6 +105,29 @@ func TestNewReleasesRateLimitExceeded(t *testing.T) {
}
}

func TestNewReleasesMaxRetry(t *testing.T) {
t.Parallel()
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Retry-After", "3660") // 61 minutes
w.WriteHeader(rateLimitExceededStatusCode)
_, _ = io.WriteString(w, `{ "error": { "message": "slow down", "status": 429 } }`)
})

server := httptest.NewServer(handler)
defer server.Close()

client := &Client{
http: http.DefaultClient,
baseURL: server.URL + "/",
autoRetry: true,
maxRetryDuration: time.Hour,
}
_, err := client.NewReleases(context.Background())
if err != ErrMaxRetryDurationExceeded {
t.Errorf("Should throw 'ErrMaxRetryDurationExceeded' error, got '%s'", err)
}
}

func TestClient_Token(t *testing.T) {
// oauth setup for valid test token
config := oauth2.Config{
Expand Down Expand Up @@ -144,14 +168,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)
Expand Down