Compare commits

..

3 Commits

Author SHA1 Message Date
agent-company e6ce6bc6c6 feat: add rate-limit retry with exponential backoff in Gitea API client
Add automatic retry logic to doRequest for HTTP 429 responses. Uses
Retry-After header when present, otherwise exponential backoff
(1s, 2s, 4s). Respects context cancellation during waits. Defaults
to 3 max retries with 1s base delay. Includes 7 new tests covering
retry success, exhaustion, Retry-After header, context cancellation,
non-429 errors, and backoff calculation.

Closes leeworks-agents/gitea-mobile#132

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-28 18:08:48 +00:00
AI-Manager 77c8e92e38 Merge pull request 'test: unit tests for SubmitReview and ApplyLabel client methods' (#130) from feature/unit-tests-submit-review-apply-label-127 into master
Build and Push / test (push) Has been cancelled
Build and Push / build (push) Has been cancelled
2026-03-28 15:03:40 +00:00
agent-company becb925456 test: add unit tests for SubmitReview and ApplyLabel client methods
Add four test functions using httptest.NewServer:
- TestApplyLabel: verifies POST request path, auth header, label IDs
  in body, and cache invalidation after success
- TestApplyLabel_Error: verifies 404 error propagation
- TestSubmitReview: verifies POST path, event/body fields, and cache
  invalidation after success
- TestSubmitReview_Error: verifies 422 error propagation

Closes leeworks-agents/gitea-mobile#127

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-28 13:08:23 +00:00
2 changed files with 384 additions and 20 deletions
+91 -20
View File
@@ -8,8 +8,11 @@ import (
"encoding/json"
"fmt"
"io"
"log/slog"
"math"
"net/http"
"sort"
"strconv"
"strings"
"sync"
"time"
@@ -27,6 +30,11 @@ type Client struct {
maxConcurrent int
// cacheTTL controls how long cache entries remain valid.
cacheTTL time.Duration
// maxRetries is the maximum number of retries for rate-limited requests.
maxRetries int
// baseRetryDelay is the initial backoff delay before the first retry.
baseRetryDelay time.Duration
}
type cacheEntry struct {
@@ -129,39 +137,102 @@ func NewClient(baseURL string) *Client {
httpClient: &http.Client{
Timeout: 30 * time.Second,
},
cache: make(map[string]*cacheEntry),
maxConcurrent: 5,
cacheTTL: 30 * time.Second,
cache: make(map[string]*cacheEntry),
maxConcurrent: 5,
cacheTTL: 30 * time.Second,
maxRetries: 3,
baseRetryDelay: 1 * time.Second,
}
}
// doRequest performs an authenticated HTTP request to the Gitea API.
// It automatically retries on HTTP 429 (rate limit) responses with
// exponential backoff, respecting the Retry-After header when present.
func (c *Client) doRequest(ctx context.Context, token, method, path string, body io.Reader) (*http.Response, error) {
url := c.baseURL + "/api/v1" + path
req, err := http.NewRequestWithContext(ctx, method, url, body)
if err != nil {
return nil, fmt.Errorf("creating request: %w", err)
}
req.Header.Set("Authorization", "token "+token)
req.Header.Set("Accept", "application/json")
// Read the body once so we can replay it on retries.
var bodyBytes []byte
if body != nil {
req.Header.Set("Content-Type", "application/json")
var err error
bodyBytes, err = io.ReadAll(body)
if err != nil {
return nil, fmt.Errorf("reading request body: %w", err)
}
}
resp, err := c.httpClient.Do(req)
if err != nil {
return nil, fmt.Errorf("executing request: %w", err)
var lastErr error
for attempt := 0; attempt <= c.maxRetries; attempt++ {
// Recreate the body reader for each attempt.
var reqBody io.Reader
if bodyBytes != nil {
reqBody = strings.NewReader(string(bodyBytes))
}
req, err := http.NewRequestWithContext(ctx, method, url, reqBody)
if err != nil {
return nil, fmt.Errorf("creating request: %w", err)
}
req.Header.Set("Authorization", "token "+token)
req.Header.Set("Accept", "application/json")
if bodyBytes != nil {
req.Header.Set("Content-Type", "application/json")
}
resp, err := c.httpClient.Do(req)
if err != nil {
return nil, fmt.Errorf("executing request: %w", err)
}
// Not rate-limited: handle normally.
if resp.StatusCode != http.StatusTooManyRequests {
if resp.StatusCode >= 400 {
defer resp.Body.Close()
respBody, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("API error %d: %s", resp.StatusCode, string(respBody))
}
return resp, nil
}
// Rate-limited (429): close body and compute retry delay.
resp.Body.Close()
if attempt == c.maxRetries {
lastErr = fmt.Errorf("API rate limit exceeded after %d retries (429)", c.maxRetries)
break
}
delay := c.retryDelay(resp, attempt)
slog.Warn("rate limited by Gitea API, retrying",
"attempt", attempt+1,
"max_retries", c.maxRetries,
"delay", delay,
"path", path,
)
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-time.After(delay):
// Continue to next attempt.
}
}
if resp.StatusCode >= 400 {
defer resp.Body.Close()
respBody, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("API error %d: %s", resp.StatusCode, string(respBody))
}
return nil, lastErr
}
return resp, nil
// retryDelay computes the delay before the next retry attempt. It uses the
// Retry-After header value (in seconds) if present, otherwise falls back to
// exponential backoff: baseRetryDelay * 2^attempt.
func (c *Client) retryDelay(resp *http.Response, attempt int) time.Duration {
if ra := resp.Header.Get("Retry-After"); ra != "" {
if seconds, err := strconv.Atoi(ra); err == nil && seconds > 0 {
return time.Duration(seconds) * time.Second
}
}
// Exponential backoff: 1s, 2s, 4s, ...
return c.baseRetryDelay * time.Duration(math.Pow(2, float64(attempt)))
}
// getFromCache returns cached data if still valid.
+293
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
)
@@ -893,6 +894,140 @@ func TestListAllPullRequests_StateFilter(t *testing.T) {
}
}
// --- Issue #127: Tests for ApplyLabel and SubmitReview ---
func TestApplyLabel(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
if r.URL.Path != "/api/v1/repos/owner1/repo1/issues/42/labels" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.Header.Get("Authorization") != "token test-token" {
t.Error("missing or wrong Authorization header")
}
var body map[string]interface{}
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Fatalf("failed to decode body: %v", err)
}
labels, ok := body["labels"].([]interface{})
if !ok {
t.Fatalf("expected labels array, got %T", body["labels"])
}
if len(labels) != 2 {
t.Errorf("expected 2 label IDs, got %d", len(labels))
}
// Verify the label IDs are correct (JSON numbers are float64).
if labels[0].(float64) != 10 || labels[1].(float64) != 20 {
t.Errorf("expected label IDs [10, 20], got %v", labels)
}
w.WriteHeader(http.StatusOK)
json.NewEncoder(w).Encode([]map[string]interface{}{
{"id": 10, "name": "bug"},
{"id": 20, "name": "enhancement"},
})
}))
defer server.Close()
c := NewClient(server.URL)
c.setCache("issues-org1", "should-be-invalidated")
err := c.ApplyLabel(context.Background(), "test-token", "owner1", "repo1", 42, []int64{10, 20})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// Verify cache was invalidated.
_, ok := c.getFromCache("issues-org1")
if ok {
t.Error("expected cache to be invalidated after ApplyLabel")
}
}
func TestApplyLabel_Error(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
fmt.Fprintln(w, `{"message":"issue not found"}`)
}))
defer server.Close()
c := NewClient(server.URL)
err := c.ApplyLabel(context.Background(), "test-token", "owner1", "repo1", 999, []int64{10})
if err == nil {
t.Fatal("expected error for 404 response, got nil")
}
if !strings.Contains(err.Error(), "404") {
t.Errorf("error should contain status code 404, got: %v", err)
}
}
func TestSubmitReview(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
if r.URL.Path != "/api/v1/repos/owner1/repo1/pulls/7/reviews" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.Header.Get("Authorization") != "token test-token" {
t.Error("missing or wrong Authorization header")
}
var body map[string]string
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Fatalf("failed to decode body: %v", err)
}
if body["event"] != "APPROVED" {
t.Errorf("expected event=APPROVED, got %q", body["event"])
}
if body["body"] != "Looks good!" {
t.Errorf("expected body='Looks good!', got %q", body["body"])
}
w.WriteHeader(http.StatusOK)
json.NewEncoder(w).Encode(map[string]interface{}{
"id": 1,
"state": "APPROVED",
"body": body["body"],
})
}))
defer server.Close()
c := NewClient(server.URL)
c.setCache("pulls-org1", "should-be-invalidated")
err := c.SubmitReview(context.Background(), "test-token", "owner1", "repo1", 7, "APPROVED", "Looks good!")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// Verify cache was invalidated.
_, ok := c.getFromCache("pulls-org1")
if ok {
t.Error("expected cache to be invalidated after SubmitReview")
}
}
func TestSubmitReview_Error(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
fmt.Fprintln(w, `{"message":"validation failed"}`)
}))
defer server.Close()
c := NewClient(server.URL)
err := c.SubmitReview(context.Background(), "test-token", "owner1", "repo1", 7, "INVALID", "")
if err == nil {
t.Fatal("expected error for 422 response, got nil")
}
if !strings.Contains(err.Error(), "422") {
t.Errorf("error should contain status code 422, got: %v", err)
}
}
func TestListAllPullRequests_Pagination(t *testing.T) {
now := time.Date(2026, 3, 28, 12, 0, 0, 0, time.UTC)
@@ -952,3 +1087,161 @@ func TestListAllPullRequests_Pagination(t *testing.T) {
t.Error("page 2: HasMore should be false")
}
}
func TestDoRequest_RateLimitRetry(t *testing.T) {
attempts := 0
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
attempts++
if attempts <= 2 {
w.Header().Set("Retry-After", "0")
w.WriteHeader(http.StatusTooManyRequests)
return
}
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, `[{"username":"test-org"}]`)
}))
defer srv.Close()
c := NewClient(srv.URL)
c.maxRetries = 3
c.baseRetryDelay = 1 * time.Millisecond // Fast for tests.
resp, err := c.doRequest(context.Background(), "test-token", "GET", "/user/orgs", nil)
if err != nil {
t.Fatalf("expected success after retries, got: %v", err)
}
resp.Body.Close()
if attempts != 3 {
t.Errorf("expected 3 attempts, got %d", attempts)
}
}
func TestDoRequest_RateLimitExhausted(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusTooManyRequests)
}))
defer srv.Close()
c := NewClient(srv.URL)
c.maxRetries = 2
c.baseRetryDelay = 1 * time.Millisecond
_, err := c.doRequest(context.Background(), "test-token", "GET", "/user/orgs", nil)
if err == nil {
t.Fatal("expected error after exhausting retries")
}
if !strings.Contains(err.Error(), "rate limit exceeded") {
t.Errorf("expected rate limit error, got: %v", err)
}
}
func TestDoRequest_RateLimitWithRetryAfterHeader(t *testing.T) {
attempts := 0
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
attempts++
if attempts == 1 {
w.Header().Set("Retry-After", "1")
w.WriteHeader(http.StatusTooManyRequests)
return
}
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, `[]`)
}))
defer srv.Close()
c := NewClient(srv.URL)
c.maxRetries = 3
c.baseRetryDelay = 1 * time.Millisecond
start := time.Now()
resp, err := c.doRequest(context.Background(), "test-token", "GET", "/user/orgs", nil)
elapsed := time.Since(start)
if err != nil {
t.Fatalf("expected success, got: %v", err)
}
resp.Body.Close()
// Retry-After: 1 means 1 second delay.
if elapsed < 900*time.Millisecond {
t.Errorf("expected at least ~1s delay from Retry-After header, got %v", elapsed)
}
}
func TestDoRequest_RateLimitCancelledContext(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Retry-After", "60")
w.WriteHeader(http.StatusTooManyRequests)
}))
defer srv.Close()
c := NewClient(srv.URL)
c.maxRetries = 3
c.baseRetryDelay = 1 * time.Millisecond
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
_, err := c.doRequest(ctx, "test-token", "GET", "/user/orgs", nil)
if err == nil {
t.Fatal("expected error from cancelled context")
}
}
func TestDoRequest_NonRateLimitErrorNotRetried(t *testing.T) {
attempts := 0
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
attempts++
w.WriteHeader(http.StatusForbidden)
fmt.Fprint(w, `{"message":"forbidden"}`)
}))
defer srv.Close()
c := NewClient(srv.URL)
c.maxRetries = 3
c.baseRetryDelay = 1 * time.Millisecond
_, err := c.doRequest(context.Background(), "test-token", "GET", "/user/orgs", nil)
if err == nil {
t.Fatal("expected error for 403")
}
if attempts != 1 {
t.Errorf("expected only 1 attempt for non-429 error, got %d", attempts)
}
}
func TestRetryDelay_WithRetryAfterHeader(t *testing.T) {
c := NewClient("https://example.com")
c.baseRetryDelay = 1 * time.Second
resp := &http.Response{Header: http.Header{}}
resp.Header.Set("Retry-After", "5")
delay := c.retryDelay(resp, 0)
if delay != 5*time.Second {
t.Errorf("expected 5s from Retry-After, got %v", delay)
}
}
func TestRetryDelay_ExponentialBackoff(t *testing.T) {
c := NewClient("https://example.com")
c.baseRetryDelay = 1 * time.Second
resp := &http.Response{Header: http.Header{}}
tests := []struct {
attempt int
want time.Duration
}{
{0, 1 * time.Second},
{1, 2 * time.Second},
{2, 4 * time.Second},
}
for _, tt := range tests {
delay := c.retryDelay(resp, tt.attempt)
if delay != tt.want {
t.Errorf("attempt %d: got %v, want %v", tt.attempt, delay, tt.want)
}
}
}