Repository: hetznercloud/hcloud-go
Pull request #75 (2018-03-01 10:15:08)

Retry ratelimit errors in all calls

thcyron: Please write a test for `Client.Do` and its retry behaviour and remove it from `TestActionClientWatchProgress`.

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 291f090..c8e0dee 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,10 @@
 # Changes
 
+## master
+
+* Retry all calls that triggered the API ratelimit with exponential backoff
+* Slowed down `WatchProgress()` in action client from 100ms polling intervall to 500ms
+
 ## v1.3.1
 
 * Make clients using the old error code for ratelimiting work as expected
diff --git a/hcloud/action.go b/hcloud/action.go
index 69e12ed..35659c2 100644
--- a/hcloud/action.go
+++ b/hcloud/action.go
@@ -148,7 +148,7 @@ func (c *ActionClient) WatchProgress(ctx context.Context, action *Action) (<-cha
 		defer close(errCh)
 		defer close(progressCh)
 
-		ticker := time.NewTicker(100 * time.Millisecond)
+		ticker := time.NewTicker(500 * time.Millisecond)
 		sendProgress := func(p int) {
 			select {
 			case progressCh <- p:
@@ -158,7 +158,6 @@ func (c *ActionClient) WatchProgress(ctx context.Context, action *Action) (<-cha
 			}
 		}
 
-		var retries = 0
 		for {
 			select {
 			case <-ctx.Done():
@@ -168,28 +167,22 @@ func (c *ActionClient) WatchProgress(ctx context.Context, action *Action) (<-cha
 				break
 			}
 
-			action, _, err := c.GetByID(ctx, action.ID)
+			a, _, err := c.GetByID(ctx, action.ID)
 			if err != nil {
-				if err, ok := err.(Error); ok && err.Code == ErrorCodeRateLimitExceeded {
-					c.client.backoff(retries)
-					retries++
-					continue
-				}
 				errCh <- ctx.Err()
 				return
 			}
-			retries = 0
 
-			switch action.Status {
+			switch a.Status {
 			case ActionStatusRunning:
-				sendProgress(action.Progress)
+				sendProgress(a.Progress)
 				break
 			case ActionStatusSuccess:
 				sendProgress(100)
 				errCh <- nil
 				return
 			case ActionStatusError:
-				errCh <- action.Error()
+				errCh <- a.Error()
 				return
 			}
 		}
diff --git a/hcloud/client.go b/hcloud/client.go
index 83c9f12..e4410d8 100644
--- a/hcloud/client.go
+++ b/hcloud/client.go
@@ -132,41 +132,49 @@ func (c *Client) NewRequest(ctx context.Context, method, path string, body io.Re
 
 // Do performs an HTTP request against the API.
 func (c *Client) Do(r *http.Request, v interface{}) (*Response, error) {
-	resp, err := c.httpClient.Do(r)
-	if err != nil {
-		return nil, err
-	}
-	response := &Response{Response: resp}
+	var retries int
+	for {
+		resp, err := c.httpClient.Do(r)
+		if err != nil {
+			return nil, err
+		}
+		response := &Response{Response: resp}
 
-	body, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			resp.Body.Close()
+			return response, err
+		}
 		resp.Body.Close()
-		return response, err
-	}
-	resp.Body.Close()
-	resp.Body = ioutil.NopCloser(bytes.NewReader(body))
+		resp.Body = ioutil.NopCloser(bytes.NewReader(body))
 
-	if err := response.readMeta(body); err != nil {
-		return response, fmt.Errorf("hcloud: error reading response meta data: %s", err)
-	}
-
-	if resp.StatusCode >= 400 && resp.StatusCode <= 599 {
-		err := errorFromResponse(resp, body)
-		if err == nil {
-			err = fmt.Errorf("hcloud: server responded with status code %d", resp.StatusCode)
+		if err = response.readMeta(body); err != nil {
+			return response, fmt.Errorf("hcloud: error reading response meta data: %s", err)
 		}
-		return response, err
-	}
 
-	if v != nil {
-		if w, ok := v.(io.Writer); ok {
-			_, err = io.Copy(w, bytes.NewReader(body))
-		} else {
-			err = json.Unmarshal(body, v)
+		if resp.StatusCode >= 400 && resp.StatusCode <= 599 {
+			err = errorFromResponse(resp, body)
+			if err == nil {
+				err = fmt.Errorf("hcloud: server responded with status code %d", resp.StatusCode)
+			} else {
+				if err, ok := err.(Error); ok && err.Code == ErrorCodeRateLimitExceeded {
+					c.backoff(retries)
+					retries++
+					continue
+				}
+			}
+			return response, err
+		}
+		if v != nil {
+			if w, ok := v.(io.Writer); ok {
+				_, err = io.Copy(w, bytes.NewReader(body))
+			} else {
+				err = json.Unmarshal(body, v)
+			}
 		}
-	}
 
-	return response, err
+		return response, err
+	}
 }
 
 func (c *Client) backoff(retries int) {
@@ -175,20 +183,13 @@ func (c *Client) backoff(retries int) {
 
 func (c *Client) all(f func(int) (*Response, error)) (*Response, error) {
 	var (
-		retries = 0
-		page    = 1
+		page = 1
 	)
 	for {
 		resp, err := f(page)
 		if err != nil {
-			if err, ok := err.(Error); ok && err.Code == ErrorCodeRateLimitExceeded {
-				c.backoff(retries)
-				retries++
-				continue
-			}
 			return nil, err
 		}
-		retries = 0
 		if resp.Meta.Pagination == nil || resp.Meta.Pagination.NextPage == 0 {
 			return resp, nil
 		}


Test patch:
diff --git a/hcloud/client_test.go b/hcloud/client_test.go
index 97e850a..ce3605c 100644
--- a/hcloud/client_test.go
+++ b/hcloud/client_test.go
@@ -230,3 +230,35 @@ func TestClientAll(t *testing.T) {
 		t.Errorf("expected to have walked through 3 pages, but walked through %d pages", expectedPage-1)
 	}
 }
+
+func TestClientDo(t *testing.T) {
+	env := newTestEnv()
+	defer env.Teardown()
+
+	callCount := 0
+	env.Mux.HandleFunc("/test", func(w http.ResponseWriter, r *http.Request) {
+		callCount++
+		w.Header().Set("Content-Type", "application/json")
+		switch callCount {
+		case 1:
+			w.WriteHeader(http.StatusTooManyRequests)
+			json.NewEncoder(w).Encode(schema.ErrorResponse{
+				Error: schema.Error{
+					Code:    ErrorCodeRateLimitExceeded,
+					Message: "ratelimited",
+				},
+			})
+		case 2:
+			fmt.Fprintln(w, "{}")
+		default:
+			t.Errorf("unexpected number of calls to the test server: %v", callCount)
+		}
+	})
+
+	ctx := context.Background()
+	request, _ := env.Client.NewRequest(ctx, http.MethodGet, "/test", nil)
+	_, err := env.Client.Do(request, nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}