fix(gitea): address review feedback on retry logic
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 23s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 30s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 45s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m30s
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 23s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 30s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 45s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m30s
- Remove dead backoff[0] element; array now only contains retry delays - Fix time.After timer leak by using time.NewTimer with timer.Stop() - Add io.LimitReader (64KB) for error body reads to bound memory allocation Addresses feedback from sonnet-review-bot, security-review-bot, and gpt-review-bot.
This commit is contained in:
+22
-10
@@ -215,23 +215,32 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
|
|||||||
}
|
}
|
||||||
return &review, nil
|
return &review, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// doGet performs an HTTP GET request with retry on 5xx errors.
|
// doGet performs an HTTP GET request with retry on 5xx errors.
|
||||||
// Retries up to 3 times with exponential backoff (1s, 2s delays).
|
// Retries up to 3 times with exponential backoff (1s, 2s delays).
|
||||||
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
||||||
const maxAttempts = 3
|
const maxAttempts = 3
|
||||||
backoff := []time.Duration{0, 1 * time.Second, 2 * time.Second}
|
// backoff[i] is the delay before attempt i+1 (i.e., after attempt i fails).
|
||||||
|
// First attempt (i=0) has no delay; retries wait 1s then 2s.
|
||||||
|
backoff := []time.Duration{1 * time.Second, 2 * time.Second}
|
||||||
|
|
||||||
|
// maxErrorBodyBytes limits how much of an error response body we read
|
||||||
|
// to protect against malicious servers sending unbounded data.
|
||||||
|
const maxErrorBodyBytes = 64 * 1024 // 64 KB
|
||||||
|
|
||||||
var lastErr error
|
var lastErr error
|
||||||
for attempt := 0; attempt < maxAttempts; attempt++ {
|
for attempt := 0; attempt < maxAttempts; attempt++ {
|
||||||
if attempt > 0 {
|
if attempt > 0 {
|
||||||
|
delay := backoff[attempt-1]
|
||||||
slog.Warn("retrying request after server error",
|
slog.Warn("retrying request after server error",
|
||||||
"attempt", attempt+1,
|
"attempt", attempt+1,
|
||||||
"url", reqURL,
|
"url", reqURL,
|
||||||
"delay", backoff[attempt].String())
|
"delay", delay.String())
|
||||||
|
|
||||||
|
timer := time.NewTimer(delay)
|
||||||
select {
|
select {
|
||||||
case <-time.After(backoff[attempt]):
|
case <-timer.C:
|
||||||
case <-ctx.Done():
|
case <-ctx.Done():
|
||||||
|
timer.Stop()
|
||||||
return nil, ctx.Err()
|
return nil, ctx.Err()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -247,17 +256,20 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
body, readErr := io.ReadAll(resp.Body)
|
|
||||||
resp.Body.Close()
|
|
||||||
|
|
||||||
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
|
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
|
||||||
if readErr != nil {
|
body, err := io.ReadAll(resp.Body)
|
||||||
return nil, readErr
|
resp.Body.Close()
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
}
|
}
|
||||||
return body, nil
|
return body, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
lastErr = &APIError{StatusCode: resp.StatusCode, Body: string(body)}
|
// Error path: limit how much we read from potentially malicious server
|
||||||
|
errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
|
||||||
|
resp.Body.Close()
|
||||||
|
|
||||||
|
lastErr = &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
|
||||||
|
|
||||||
// Only retry on 5xx server errors
|
// Only retry on 5xx server errors
|
||||||
if resp.StatusCode < 500 || resp.StatusCode >= 600 {
|
if resp.StatusCode < 500 || resp.StatusCode >= 600 {
|
||||||
|
|||||||
Reference in New Issue
Block a user