From 1aa556829dbfa2217eefdd68ee46d5551efab9b4 Mon Sep 17 00:00:00 2001 From: yumosx Date: Thu, 26 Feb 2026 09:27:51 +0800 Subject: [PATCH] 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 --- pkg/skills/installer.go | 39 ++----------- pkg/utils/http_retry.go | 55 +++++++++++++++++++ .../http_retry_test.go} | 37 ++++++------- 3 files changed, 75 insertions(+), 56 deletions(-) create mode 100644 pkg/utils/http_retry.go rename pkg/{skills/installer_test.go => utils/http_retry_test.go} (80%) diff --git a/pkg/skills/installer.go b/pkg/skills/installer.go index 2650e55d6..f9b5705f1 100644 --- a/pkg/skills/installer.go +++ b/pkg/skills/installer.go @@ -9,6 +9,8 @@ import ( "os" "path/filepath" "time" + + "github.com/sipeed/picoclaw/pkg/utils" ) type SkillInstaller struct { @@ -23,39 +25,6 @@ type AvailableSkill struct { Tags []string `json:"tags"` } -const maxRetries = 3 - -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 { - time.Sleep(time.Second * time.Duration(i+1)) - } - } - return resp, err -} - func NewSkillInstaller(workspace string) *SkillInstaller { return &SkillInstaller{ workspace: workspace, @@ -77,7 +46,7 @@ func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) er return fmt.Errorf("failed to create request: %w", err) } - resp, err := doRequestWithRetry(client, req) + resp, err := utils.DoRequestWithRetry(client, req) if err != nil { return fmt.Errorf("failed to fetch skill: %w", err) } @@ -127,7 +96,7 @@ func (si *SkillInstaller) ListAvailableSkills(ctx context.Context) ([]AvailableS return nil, fmt.Errorf("failed to create request: %w", err) } - resp, err := doRequestWithRetry(client, req) + resp, err := utils.DoRequestWithRetry(client, req) if err != nil { return nil, fmt.Errorf("failed to fetch skills list: %w", err) } diff --git a/pkg/utils/http_retry.go b/pkg/utils/http_retry.go new file mode 100644 index 000000000..1ad904064 --- /dev/null +++ b/pkg/utils/http_retry.go @@ -0,0 +1,55 @@ +package utils + +import ( + "context" + "fmt" + "net/http" + "time" +) + +const maxRetries = 3 + +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(), time.Second*time.Duration(i+1)); err != nil { + return nil, fmt.Errorf("failed to sleep: %w", err) + } + } + } + return resp, err +} + +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 + } +} diff --git a/pkg/skills/installer_test.go b/pkg/utils/http_retry_test.go similarity index 80% rename from pkg/skills/installer_test.go rename to pkg/utils/http_retry_test.go index 9ce6ddcd4..51af33038 100644 --- a/pkg/skills/installer_test.go +++ b/pkg/utils/http_retry_test.go @@ -1,4 +1,4 @@ -package skills +package utils import ( "net/http" @@ -7,6 +7,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDoRequestWithRetry(t *testing.T) { @@ -48,14 +49,6 @@ func TestDoRequestWithRetry(t *testing.T) { wantSuccess: false, wantAttempts: 3, }, - { - name: "non-ok-status-code", - serverBehavior: func(server *httptest.Server) int { - return 4 - }, - wantSuccess: false, - wantAttempts: 3, - }, } for _, tc := range testcases { @@ -71,21 +64,23 @@ func TestDoRequestWithRetry(t *testing.T) { w.Write([]byte("success")) })) - defer server.Close() + t.Cleanup(func() { + server.Close() + }) client := &http.Client{Timeout: 5 * time.Second} - req, err := http.NewRequest("GET", server.URL, nil) - assert.NoError(t, err) + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) - resp, err := doRequestWithRetry(client, req) + resp, err := DoRequestWithRetry(client, req) if tc.wantSuccess { - assert.NoError(t, err) - assert.NotNil(t, resp) + require.NoError(t, err) + require.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) resp.Body.Close() } else { - assert.NotNil(t, resp) + require.NotNil(t, resp) assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) resp.Body.Close() } @@ -120,12 +115,12 @@ func TestDoRequestWithRetry_Delay(t *testing.T) { defer server.Close() client := &http.Client{Timeout: 10 * time.Second} - req, err := http.NewRequest("GET", server.URL, nil) - assert.NoError(t, err) + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) - resp, err := doRequestWithRetry(client, req) - assert.NoError(t, err) - assert.NotNil(t, resp) + resp, err := DoRequestWithRetry(client, req) + require.NoError(t, err) + require.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) resp.Body.Close()