fix(retry): address review edge cases in notices and deadline handling
This commit is contained in:
parent
cb1d727057
commit
f8720e16ed
4 changed files with 84 additions and 2 deletions
|
|
@ -74,7 +74,7 @@ func RunToolLoop(
|
||||||
if existingNotify != nil {
|
if existingNotify != nil {
|
||||||
existingNotify(notice)
|
existingNotify(notice)
|
||||||
}
|
}
|
||||||
if config.RetryNotice != nil && channel != "" && chatID != "" {
|
if config.RetryNotice != nil {
|
||||||
config.RetryNotice(utils.FormatLLMRetryNotice(notice))
|
config.RetryNotice(utils.FormatLLMRetryNotice(notice))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -81,6 +81,50 @@ func TestRunToolLoop_TransientRetry(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunToolLoop_TransientRetry_NoticeWithoutChannelContext(t *testing.T) {
|
||||||
|
provider := &flakyToolLoopProvider{
|
||||||
|
errors: []error{
|
||||||
|
fmt.Errorf("API request failed: status: 502 body: bad gateway"),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
notices := make([]string, 0, 1)
|
||||||
|
cfg := ToolLoopConfig{
|
||||||
|
Provider: provider,
|
||||||
|
Model: "test-model",
|
||||||
|
MaxIterations: 1,
|
||||||
|
RetryPolicy: &utils.RetryPolicy{
|
||||||
|
AttemptTimeouts: []time.Duration{time.Second, time.Second},
|
||||||
|
},
|
||||||
|
RetryNotice: func(content string) {
|
||||||
|
notices = append(notices, content)
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result, err := RunToolLoop(
|
||||||
|
context.Background(),
|
||||||
|
cfg,
|
||||||
|
[]providers.Message{{Role: "user", Content: "hello"}},
|
||||||
|
"",
|
||||||
|
"",
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if result == nil || result.Content != "ok" {
|
||||||
|
t.Fatalf("unexpected result: %+v", result)
|
||||||
|
}
|
||||||
|
if provider.calls != 2 {
|
||||||
|
t.Fatalf("provider.calls = %d, want 2", provider.calls)
|
||||||
|
}
|
||||||
|
if len(notices) != 1 {
|
||||||
|
t.Fatalf("notices = %d, want 1", len(notices))
|
||||||
|
}
|
||||||
|
if !strings.Contains(strings.ToLower(notices[0]), "retry") {
|
||||||
|
t.Fatalf("notice = %q, want retry hint", notices[0])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestRunToolLoop_NonRetryableError_NoRetry(t *testing.T) {
|
func TestRunToolLoop_NonRetryableError_NoRetry(t *testing.T) {
|
||||||
provider := &flakyToolLoopProvider{
|
provider := &flakyToolLoopProvider{
|
||||||
errors: []error{
|
errors: []error{
|
||||||
|
|
|
||||||
|
|
@ -120,7 +120,10 @@ func DoWithRetry[T any](ctx context.Context, policy RetryPolicy, fn func(context
|
||||||
if attemptTimeout := policy.AttemptTimeouts[attempt]; attemptTimeout > 0 {
|
if attemptTimeout := policy.AttemptTimeouts[attempt]; attemptTimeout > 0 {
|
||||||
timeout, ok := boundedAttemptTimeout(runCtx, attemptTimeout)
|
timeout, ok := boundedAttemptTimeout(runCtx, attemptTimeout)
|
||||||
if !ok {
|
if !ok {
|
||||||
return zero, runCtx.Err()
|
if err := runCtx.Err(); err != nil {
|
||||||
|
return zero, err
|
||||||
|
}
|
||||||
|
return zero, context.DeadlineExceeded
|
||||||
}
|
}
|
||||||
attemptCtx, cancelAttempt = context.WithTimeout(runCtx, timeout)
|
attemptCtx, cancelAttempt = context.WithTimeout(runCtx, timeout)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,19 @@ import (
|
||||||
"github.com/sipeed/picoclaw/pkg/providers"
|
"github.com/sipeed/picoclaw/pkg/providers"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
type staleDeadlineContext struct {
|
||||||
|
context.Context
|
||||||
|
deadline time.Time
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c staleDeadlineContext) Deadline() (time.Time, bool) {
|
||||||
|
return c.deadline, true
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c staleDeadlineContext) Err() error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func TestLLMRetry_ClassifyRetryDecision_429WithRetryAfter(t *testing.T) {
|
func TestLLMRetry_ClassifyRetryDecision_429WithRetryAfter(t *testing.T) {
|
||||||
err := errors.New("API request failed:\n Status: 429\n Retry-After: 7")
|
err := errors.New("API request failed:\n Status: 429\n Retry-After: 7")
|
||||||
decision := ClassifyRetryDecision(err)
|
decision := ClassifyRetryDecision(err)
|
||||||
|
|
@ -163,3 +176,25 @@ func TestLLMRetry_ExtractRetryAfter_HTTPDate(t *testing.T) {
|
||||||
t.Fatalf("delay = %v, want 1m", delay)
|
t.Fatalf("delay = %v, want 1m", delay)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestLLMRetry_DoWithRetry_ExpiredDeadlineReturnsDeadlineExceeded(t *testing.T) {
|
||||||
|
ctx := staleDeadlineContext{
|
||||||
|
Context: context.Background(),
|
||||||
|
deadline: time.Now().Add(-time.Second),
|
||||||
|
}
|
||||||
|
|
||||||
|
calls := 0
|
||||||
|
_, err := DoWithRetry(ctx, RetryPolicy{
|
||||||
|
AttemptTimeouts: []time.Duration{time.Second},
|
||||||
|
}, func(context.Context) (string, error) {
|
||||||
|
calls++
|
||||||
|
return "ok", nil
|
||||||
|
})
|
||||||
|
|
||||||
|
if !errors.Is(err, context.DeadlineExceeded) {
|
||||||
|
t.Fatalf("err = %v, want context deadline exceeded", err)
|
||||||
|
}
|
||||||
|
if calls != 0 {
|
||||||
|
t.Fatalf("calls = %d, want 0", calls)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue