Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion providers/aws/recommendations/ratelimiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,13 @@ func (r *RateLimiter) newOperation() *RateLimiter {
return NewRateLimiterWithOptions(r.baseDelay, r.maxDelay, r.maxRetries)
}

// Wait implements exponential backoff delay.
// Wait implements exponential backoff delay. An already-canceled or expired
// context is reported on every attempt, including the first, so callers never
// proceed to an API call with a dead context.
func (r *RateLimiter) Wait(ctx context.Context) error {
if err := ctx.Err(); err != nil {
return err
}
if r.retryCount == 0 {
// No delay for first attempt
return nil
Expand Down
53 changes: 53 additions & 0 deletions providers/aws/recommendations/ratelimiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/service/costexplorer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -126,6 +127,58 @@ func TestWait_ContextTimeout(t *testing.T) {
assert.Equal(t, context.DeadlineExceeded, err)
}

// TestWait_FirstAttemptHonorsCanceledContext pins issue #16: Wait must report
// an already-canceled context even on the first attempt (retryCount == 0),
// where no backoff delay applies.
func TestWait_FirstAttemptHonorsCanceledContext(t *testing.T) {
for _, tc := range []struct {
name string
ctx func() context.Context
want error
}{
{"canceled", func() context.Context {
ctx, cancel := context.WithCancel(context.Background())
cancel()
return ctx
}, context.Canceled},
{"deadline exceeded", func() context.Context {
ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second))
t.Cleanup(cancel)
return ctx
}, context.DeadlineExceeded},
} {
t.Run(tc.name, func(t *testing.T) {
limiter := NewRateLimiter()
require.Equal(t, 0, limiter.GetRetryCount())

err := limiter.Wait(tc.ctx())

assert.ErrorIs(t, err, tc.want)
assert.Equal(t, 0, limiter.GetRetryCount(), "Wait must not change retry state")
})
}
}

// TestFetchRIPageWithRetry_PreCanceledContextSkipsCostExplorer covers the
// caller-level effect of issue #16: with no shared semaphore attached to ctx
// (so concurrency.Acquire is a no-op), a pre-canceled context must stop at the
// rate limiter and never reach Cost Explorer.
func TestFetchRIPageWithRetry_PreCanceledContextSkipsCostExplorer(t *testing.T) {
mockAPI := &mockCostExplorerAPI{
riRecommendations: &costexplorer.GetReservationPurchaseRecommendationOutput{},
}
client := NewClientWithAPI(mockAPI, "us-east-1")

ctx, cancel := context.WithCancel(context.Background())
cancel()

out, err := client.fetchRIPageWithRetry(ctx, &costexplorer.GetReservationPurchaseRecommendationInput{})

require.ErrorIs(t, err, context.Canceled)
assert.Nil(t, out)
assert.Empty(t, mockAPI.riCalls, "a pre-canceled context must not reach Cost Explorer")
}

func TestShouldRetry_NoError(t *testing.T) {
limiter := NewRateLimiter()

Expand Down
Loading