From 17ef25dcf4cbaf2d31311eed835425ed1302fc2e Mon Sep 17 00:00:00 2001 From: yumosx Date: Mon, 16 Feb 2026 12:28:53 +0800 Subject: [PATCH] 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. --- pkg/skills/installer.go | 21 +++++- pkg/skills/installer_test.go | 133 +++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 pkg/skills/installer_test.go diff --git a/pkg/skills/installer.go b/pkg/skills/installer.go index a3263c525..bdc39a40f 100644 --- a/pkg/skills/installer.go +++ b/pkg/skills/installer.go @@ -30,6 +30,23 @@ type BuiltinSkill struct { Enabled bool `json:"enabled"` } +const maxRetries = 3 + +func doRequestWithRetry(client *http.Client, req *http.Request) (*http.Response, error) { + var resp *http.Response + var err error + for i := range maxRetries { + resp, err = client.Do(req) + if err == nil && resp.StatusCode == http.StatusOK { + 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, @@ -51,7 +68,7 @@ func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) er return fmt.Errorf("failed to create request: %w", err) } - resp, err := client.Do(req) + resp, err := doRequestWithRetry(client, req) if err != nil { return fmt.Errorf("failed to fetch skill: %w", err) } @@ -101,7 +118,7 @@ func (si *SkillInstaller) ListAvailableSkills(ctx context.Context) ([]AvailableS return nil, fmt.Errorf("failed to create request: %w", err) } - resp, err := client.Do(req) + resp, err := doRequestWithRetry(client, req) if err != nil { return nil, fmt.Errorf("failed to fetch skills list: %w", err) } diff --git a/pkg/skills/installer_test.go b/pkg/skills/installer_test.go new file mode 100644 index 000000000..9ce6ddcd4 --- /dev/null +++ b/pkg/skills/installer_test.go @@ -0,0 +1,133 @@ +package skills + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestDoRequestWithRetry(t *testing.T) { + testcases := []struct { + name string + serverBehavior func(*httptest.Server) int + wantSuccess bool + wantAttempts int + }{ + { + name: "success-on-first-attempt", + serverBehavior: func(server *httptest.Server) int { + return 0 + }, + wantSuccess: true, + wantAttempts: 1, + }, + { + name: "success-on-second-attempt", + serverBehavior: func(server *httptest.Server) int { + return 1 + }, + wantSuccess: true, + wantAttempts: 2, + }, + { + name: "success-on-third-attempt", + serverBehavior: func(server *httptest.Server) int { + return 2 + }, + wantSuccess: true, + wantAttempts: 3, + }, + { + name: "fail-all-attempts", + serverBehavior: func(server *httptest.Server) int { + return 4 + }, + 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 { + t.Run(tc.name, func(t *testing.T) { + attempts := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + if attempts <= tc.serverBehavior(nil) { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusOK) + w.Write([]byte("success")) + })) + + defer server.Close() + + client := &http.Client{Timeout: 5 * time.Second} + req, err := http.NewRequest("GET", server.URL, nil) + assert.NoError(t, err) + + resp, err := doRequestWithRetry(client, req) + + if tc.wantSuccess { + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() + } else { + assert.NotNil(t, resp) + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + resp.Body.Close() + } + + assert.Equal(t, tc.wantAttempts, attempts) + }) + } +} + +func TestDoRequestWithRetry_Delay(t *testing.T) { + var start time.Time + delays := []time.Duration{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if len(delays) == 0 { + delays = append(delays, 0) + w.WriteHeader(http.StatusInternalServerError) + return + } + if len(delays) == 1 { + start = time.Now() + delays = append(delays, 0) + w.WriteHeader(http.StatusInternalServerError) + return + } + if len(delays) == 2 { + elapsed := time.Since(start) + delays = append(delays, elapsed) + w.WriteHeader(http.StatusOK) + w.Write([]byte("success")) + } + })) + defer server.Close() + + client := &http.Client{Timeout: 10 * time.Second} + req, err := http.NewRequest("GET", server.URL, nil) + assert.NoError(t, err) + + resp, err := doRequestWithRetry(client, req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() + + assert.GreaterOrEqual(t, delays[2], time.Second) +}