picoclaw/pkg/utils/http_retry.go

116 lines
2.3 KiB
Go
Raw Normal View History

feat(skills): add retry for HTTP requests in skill installer (#261) * feat(skills): add retry mechanism for HTTP requests Implement a retry mechanism with exponential backoff for HTTP requests in the skill installer. This improves reliability when fetching skills from GitHub by automatically retrying failed requests up to 3 times. Add comprehensive tests to verify retry behavior under different scenarios including success on different attempts and proper delay between retries. * fix: improve http request retry logic with status code checks Add shouldRetry helper function to determine retryable status codes. Close response body between retry attempts and break early for non-retryable status codes. * refactor: remove unused BuiltinSkill struct The struct was not being used anywhere in the codebase, so it's safe to remove it to reduce clutter and improve maintainability. * refactor(http): move retry logic to utils package Extract HTTP retry functionality from skills package to utils for better reusability Add context-aware sleep function and comprehensive tests * refactor(http): extract retry delay unit to variable Extract hardcoded retry delay unit to a variable for better testability and flexibility. Update tests to use milliseconds for faster execution while maintaining the same behavior. * test(http_retry): remove t.Parallel from test cases * test(http_retry): remove redundant test cases for retry success The removed test cases for success on second and third attempts were redundant since the retry logic is already covered by other tests. This simplifies the test suite while maintaining coverage.
2026-02-26 09:35:26 +00:00
package utils
import (
"context"
"fmt"
"net/http"
"strconv"
feat(skills): add retry for HTTP requests in skill installer (#261) * feat(skills): add retry mechanism for HTTP requests Implement a retry mechanism with exponential backoff for HTTP requests in the skill installer. This improves reliability when fetching skills from GitHub by automatically retrying failed requests up to 3 times. Add comprehensive tests to verify retry behavior under different scenarios including success on different attempts and proper delay between retries. * fix: improve http request retry logic with status code checks Add shouldRetry helper function to determine retryable status codes. Close response body between retry attempts and break early for non-retryable status codes. * refactor: remove unused BuiltinSkill struct The struct was not being used anywhere in the codebase, so it's safe to remove it to reduce clutter and improve maintainability. * refactor(http): move retry logic to utils package Extract HTTP retry functionality from skills package to utils for better reusability Add context-aware sleep function and comprehensive tests * refactor(http): extract retry delay unit to variable Extract hardcoded retry delay unit to a variable for better testability and flexibility. Update tests to use milliseconds for faster execution while maintaining the same behavior. * test(http_retry): remove t.Parallel from test cases * test(http_retry): remove redundant test cases for retry success The removed test cases for success on second and third attempts were redundant since the retry logic is already covered by other tests. This simplifies the test suite while maintaining coverage.
2026-02-26 09:35:26 +00:00
"time"
)
const maxRetries = 3
2026-03-30 14:08:21 +00:00
var (
retryDelayUnit = time.Second
maxRetrySleepDuration = 1 * time.Minute
)
feat(skills): add retry for HTTP requests in skill installer (#261) * feat(skills): add retry mechanism for HTTP requests Implement a retry mechanism with exponential backoff for HTTP requests in the skill installer. This improves reliability when fetching skills from GitHub by automatically retrying failed requests up to 3 times. Add comprehensive tests to verify retry behavior under different scenarios including success on different attempts and proper delay between retries. * fix: improve http request retry logic with status code checks Add shouldRetry helper function to determine retryable status codes. Close response body between retry attempts and break early for non-retryable status codes. * refactor: remove unused BuiltinSkill struct The struct was not being used anywhere in the codebase, so it's safe to remove it to reduce clutter and improve maintainability. * refactor(http): move retry logic to utils package Extract HTTP retry functionality from skills package to utils for better reusability Add context-aware sleep function and comprehensive tests * refactor(http): extract retry delay unit to variable Extract hardcoded retry delay unit to a variable for better testability and flexibility. Update tests to use milliseconds for faster execution while maintaining the same behavior. * test(http_retry): remove t.Parallel from test cases * test(http_retry): remove redundant test cases for retry success The removed test cases for success on second and third attempts were redundant since the retry logic is already covered by other tests. This simplifies the test suite while maintaining coverage.
2026-02-26 09:35:26 +00:00
func shouldRetry(statusCode int) bool {
return statusCode == http.StatusTooManyRequests ||
statusCode >= 500
}
func DoRequestWithRetry(client *http.Client, req *http.Request) (*http.Response, error) {
var resp *http.Response
var err error
for i := range maxRetries {
if i > 0 && resp != nil {
resp.Body.Close()
}
resp, err = client.Do(req)
if err == nil {
if resp.StatusCode == http.StatusOK {
break
}
if !shouldRetry(resp.StatusCode) {
break
}
}
if i < maxRetries-1 {
if err = sleepWithCtx(req.Context(), retryDelayForAttempt(resp, i)); err != nil {
fix(tools): close resp.Body on retry cancel and cache http.Client instances (#940) * fix(tools): close resp.Body on retry cancel and cache http.Client instances Fix resp.Body leak in DoRequestWithRetry where req.Body (request) was incorrectly closed instead of resp.Body (response) on context cancel. Cache http.Client on web search/fetch provider structs and channel adapters (WeCom, LINE) to avoid per-call allocation overhead. * fix(channels): preserve original http client timeouts for LINE and WeCom Split LINE single 60s client into infoClient (10s) for bot info lookups and apiClient (30s) for messaging API calls. Lower WeCom cached client base timeout from 60s to 30s (matching uploadMedia), and ensure it is always >= the configured ReplyTimeout so the per-request context deadline remains the effective limit. * refactor(tools): extract timeout consts and deduplicate WebFetchTool constructors Address PR review feedback from xiaket: - Define searchTimeout, perplexityTimeout, fetchTimeout, defaultMaxChars, and maxRedirects as package-level consts instead of magic numbers. - Remove misleading "No proxy" comment in NewWebFetchTool. - Deduplicate NewWebFetchTool by delegating to NewWebFetchToolWithProxy. * test(utils): add context cancellation test for DoRequestWithRetry Verify that resp.Body is properly closed when the context is canceled during retry sleep, covering the C8 resp.Body leak fix. * fix(utils): close resp in test to satisfy bodyclose linter * fix(utils): eliminate flakiness in context cancellation retry test Synchronize cancellation using an onRoundTrip callback from the transport wrapper instead of a timing-based context timeout. This ensures the first client.Do completes before cancel fires, so cancellation always hits during sleepWithCtx.
2026-03-01 05:55:46 +00:00
if resp != nil {
resp.Body.Close()
}
feat(skills): add retry for HTTP requests in skill installer (#261) * feat(skills): add retry mechanism for HTTP requests Implement a retry mechanism with exponential backoff for HTTP requests in the skill installer. This improves reliability when fetching skills from GitHub by automatically retrying failed requests up to 3 times. Add comprehensive tests to verify retry behavior under different scenarios including success on different attempts and proper delay between retries. * fix: improve http request retry logic with status code checks Add shouldRetry helper function to determine retryable status codes. Close response body between retry attempts and break early for non-retryable status codes. * refactor: remove unused BuiltinSkill struct The struct was not being used anywhere in the codebase, so it's safe to remove it to reduce clutter and improve maintainability. * refactor(http): move retry logic to utils package Extract HTTP retry functionality from skills package to utils for better reusability Add context-aware sleep function and comprehensive tests * refactor(http): extract retry delay unit to variable Extract hardcoded retry delay unit to a variable for better testability and flexibility. Update tests to use milliseconds for faster execution while maintaining the same behavior. * test(http_retry): remove t.Parallel from test cases * test(http_retry): remove redundant test cases for retry success The removed test cases for success on second and third attempts were redundant since the retry logic is already covered by other tests. This simplifies the test suite while maintaining coverage.
2026-02-26 09:35:26 +00:00
return nil, fmt.Errorf("failed to sleep: %w", err)
}
}
}
return resp, err
}
func retryDelayForAttempt(resp *http.Response, attempt int) time.Duration {
fallback := retryDelayUnit * time.Duration(attempt+1)
if resp == nil || resp.StatusCode != http.StatusTooManyRequests {
return clampRetryDelay(fallback)
}
retryAfter := resp.Header.Get("Retry-After")
if retryAfter == "" {
return clampRetryDelay(fallback)
}
if delay, ok := numericRetryAfterDelay(retryAfter); ok {
return delay
}
if when, err := http.ParseTime(retryAfter); err == nil {
delay := time.Until(when)
if serverDate, err := http.ParseTime(resp.Header.Get("Date")); err == nil {
delay = when.Sub(serverDate)
}
if delay < 0 {
return 0
}
return clampRetryDelay(delay)
}
return clampRetryDelay(fallback)
}
func numericRetryAfterDelay(retryAfter string) (time.Duration, bool) {
seconds, err := strconv.ParseInt(retryAfter, 10, 64)
if err != nil || seconds < 0 {
return 0, false
}
maxSeconds := int64(maxRetrySleepDuration / time.Second)
if seconds > maxSeconds {
return maxRetrySleepDuration, true
}
return clampRetryDelay(time.Duration(seconds) * time.Second), true
}
func clampRetryDelay(delay time.Duration) time.Duration {
if delay <= 0 {
return 0
}
if delay > maxRetrySleepDuration {
return maxRetrySleepDuration
}
return delay
}
feat(skills): add retry for HTTP requests in skill installer (#261) * feat(skills): add retry mechanism for HTTP requests Implement a retry mechanism with exponential backoff for HTTP requests in the skill installer. This improves reliability when fetching skills from GitHub by automatically retrying failed requests up to 3 times. Add comprehensive tests to verify retry behavior under different scenarios including success on different attempts and proper delay between retries. * fix: improve http request retry logic with status code checks Add shouldRetry helper function to determine retryable status codes. Close response body between retry attempts and break early for non-retryable status codes. * refactor: remove unused BuiltinSkill struct The struct was not being used anywhere in the codebase, so it's safe to remove it to reduce clutter and improve maintainability. * refactor(http): move retry logic to utils package Extract HTTP retry functionality from skills package to utils for better reusability Add context-aware sleep function and comprehensive tests * refactor(http): extract retry delay unit to variable Extract hardcoded retry delay unit to a variable for better testability and flexibility. Update tests to use milliseconds for faster execution while maintaining the same behavior. * test(http_retry): remove t.Parallel from test cases * test(http_retry): remove redundant test cases for retry success The removed test cases for success on second and third attempts were redundant since the retry logic is already covered by other tests. This simplifies the test suite while maintaining coverage.
2026-02-26 09:35:26 +00:00
func sleepWithCtx(ctx context.Context, d time.Duration) error {
timer := time.NewTimer(d)
defer timer.Stop()
select {
case <-ctx.Done():
return ctx.Err()
case <-timer.C:
return nil
}
}