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.
This commit is contained in:
parent
1d748fb742
commit
17ef25dcf4
2 changed files with 152 additions and 2 deletions
|
|
@ -30,6 +30,23 @@ type BuiltinSkill struct {
|
||||||
Enabled bool `json:"enabled"`
|
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 {
|
func NewSkillInstaller(workspace string) *SkillInstaller {
|
||||||
return &SkillInstaller{
|
return &SkillInstaller{
|
||||||
workspace: workspace,
|
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)
|
return fmt.Errorf("failed to create request: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
resp, err := client.Do(req)
|
resp, err := doRequestWithRetry(client, req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to fetch skill: %w", err)
|
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)
|
return nil, fmt.Errorf("failed to create request: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
resp, err := client.Do(req)
|
resp, err := doRequestWithRetry(client, req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to fetch skills list: %w", err)
|
return nil, fmt.Errorf("failed to fetch skills list: %w", err)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
133
pkg/skills/installer_test.go
Normal file
133
pkg/skills/installer_test.go
Normal file
|
|
@ -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)
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue