Repository: jomei/notionapi
Pull request #118 (2022-12-02 18:18:15)

Handle 429 errors
#116 

dxaviud: > @dxaviud I'm using this library too, and I'm looking forward to having the 429 errors handled. One question: is it intentional that you don't limit the number of retries?

@hariso Good question, I haven't thought about it too much because I don't know how the maintainers want to handle that. I think returning a special error after, say, 3 failed retries would be reasonable, but there are likely other viable solutions. What do you think?

@jomei Does that sound reasonable?
hariso: > @hariso Good question, I haven't thought about it too much because I don't know how the maintainers want to handle that. I think returning a special error after, say, 3 failed retries would be reasonable, but there are likely other viable solutions. What do you think?

That sounds good to me (and would be nice if the number of retries was configurable). 
dxaviud: > That sounds good to me (and would be nice if the number of retries was configurable).

I made the change, didn't implement the configuration of number of retries though since that's a bit out of the scope of this PR.
hariso: > didn't implement the configuration of number of retries though since that's a bit out of the scope of this PR.

Sounds good to me!

Patch:
diff --git a/client.go b/client.go
index 025b33e..68e9580 100644
--- a/client.go
+++ b/client.go
@@ -10,12 +10,14 @@ import (
 	"net/url"
 	"reflect"
 	"strconv"
+	"time"
 )
 
 const (
 	apiURL        = "https://api.notion.com"
 	apiVersion    = "v1"
 	notionVersion = "2022-06-28"
+	maxRetries    = 3
 )
 
 type Token string
@@ -33,6 +35,8 @@ type Client struct {
 	apiVersion    string
 	notionVersion string
 
+	maxRetries int
+
 	Token Token
 
 	Database DatabaseService
@@ -54,6 +58,7 @@ func NewClient(token Token, opts ...ClientOption) *Client {
 		baseUrl:       u,
 		apiVersion:    apiVersion,
 		notionVersion: notionVersion,
+		maxRetries:    maxRetries,
 	}
 
 	c.Database = &DatabaseClient{apiClient: c}
@@ -84,6 +89,13 @@ func WithVersion(version string) ClientOption {
 	}
 }
 
+// WithRetry overrides the default number of max retry attempts on 429 errors
+func WithRetry(retries int) ClientOption {
+	return func(c *Client) {
+		c.maxRetries = retries
+	}
+}
+
 func (c *Client) request(ctx context.Context, method string, urlStr string, queryParams map[string]string, requestBody interface{}) (*http.Response, error) {
 	u, err := c.baseUrl.Parse(fmt.Sprintf("%s/%s", c.apiVersion, urlStr))
 	if err != nil {
@@ -115,9 +127,34 @@ func (c *Client) request(ctx context.Context, method string, urlStr string, quer
 	req.Header.Add("Notion-Version", c.notionVersion)
 	req.Header.Add("Content-Type", "application/json")
 
-	res, err := c.httpClient.Do(req.WithContext(ctx))
-	if err != nil {
-		return nil, err
+	failedAttempts := 0
+	var res *http.Response
+	for {
+		var err error
+		res, err = c.httpClient.Do(req.WithContext(ctx))
+		if err != nil {
+			return nil, err
+		}
+
+		if res.StatusCode != http.StatusTooManyRequests {
+			break
+		}
+
+		failedAttempts++
+		if failedAttempts == c.maxRetries {
+			return nil, fmt.Errorf("Retry request with 429 response failed after %d retries", failedAttempts)
+		}
+		// https://developers.notion.com/reference/request-limits#rate-limits
+		retryAfter := res.Header["Retry-After"][0]
+		waitSeconds, err := strconv.Atoi(retryAfter)
+		if err != nil {
+			break // should not happen
+		}
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		case <-time.After(time.Duration(waitSeconds) * time.Second):
+		}
 	}
 
 	if res.StatusCode != http.StatusOK {
diff --git a/object.go b/object.go
index e3dc2e9..5c971cb 100644
--- a/object.go
+++ b/object.go
@@ -185,8 +185,4 @@ func (pID PropertyID) String() string {
 	return string(pID)
 }
 
-type Status struct {
-	ID    ObjectID `json:"id"`
-	Name  string   `json:"name"`
-	Color Color    `json:"color"`
-}
+type Status = Option


Test patch:
diff --git a/client_test.go b/client_test.go
index dc96738..139c0bd 100644
--- a/client_test.go
+++ b/client_test.go
@@ -1,9 +1,12 @@
 package notionapi_test
 
 import (
+	"context"
 	"net/http"
 	"os"
 	"testing"
+
+	"github.com/jomei/notionapi"
 )
 
 // RoundTripFunc .
@@ -23,7 +26,7 @@ func newTestClient(fn RoundTripFunc) *http.Client {
 
 // newMockedClient returns *http.Client which responds with content from given file
 func newMockedClient(t *testing.T, requestMockFile string, statusCode int) *http.Client {
-	return newTestClient(func(req *http.Request) *http.Response {
+	return newTestClient(func(*http.Request) *http.Response {
 		b, err := os.Open(requestMockFile)
 		if err != nil {
 			t.Fatal(err)
@@ -37,3 +40,21 @@ func newMockedClient(t *testing.T, requestMockFile string, statusCode int) *http
 		return resp
 	})
 }
+
+func TestRateLimitRetryError(t *testing.T) {
+	c := newTestClient(func(*http.Request) *http.Response {
+		return &http.Response{
+			StatusCode: http.StatusTooManyRequests,
+			Header:     http.Header{"Retry-After": []string{"0"}},
+		}
+	})
+	client := notionapi.NewClient("some_token", notionapi.WithHTTPClient(c), notionapi.WithRetry(2))
+	_, err := client.Block.Get(context.Background(), notionapi.BlockID("some_block_id"))
+	if err == nil {
+		t.Errorf("Get() error = %v", err)
+	}
+	wantErr := "Retry request with 429 response failed after 2 retries"
+	if err.Error() != wantErr {
+		t.Errorf("Get() error = %v, wantErr %s", err, wantErr)
+	}
+}