diff --git a/docs/issues/012-coingecko-rate-limiting.md b/docs/issues/012-coingecko-rate-limiting.md new file mode 100644 index 0000000..fdcde74 --- /dev/null +++ b/docs/issues/012-coingecko-rate-limiting.md @@ -0,0 +1,315 @@ +--- +status: done +branch: feat/012-coingecko-rate-limiting +--- + +# Slice 12 — CoinGecko Rate Limiting + +New runtime behaviour: the app must stay within the free-tier limit of ~30 req/min. + +## Context + +Slices 1–11 are DONE. The app has a working Markets tab with auto-refresh (every 60s), manual refresh (`r` key), and status bar states (Synced, Stale, Refreshing, error, loading). The `HTTPClient` makes two types of requests: `FetchMarkets` on initial seed and `FetchPrices` on refreshes. A `refreshing` mutex flag prevents concurrent requests at the UI level. However, there is no protection against CoinGecko's ~30 req/min demo-tier rate limit, no 429 error handling, and no backoff strategy. + +## Scope + +From the roadmap: + +1. Implement request throttling in `internal/api/coingecko.go` +2. Detect 429 responses and back off when rate-limited +3. Extend the `r` no-op to also suppress when rate-limited +4. Surface rate-limit status in the status bar (e.g. `Rate limited — retrying in Xs`) +5. TDD: throttle logic, backoff behaviour, status bar state for rate-limited condition + +## Data model + +No SQL schema changes. New application-level types only. + +## Files to create/modify + +### 1. `internal/api/coingecko.go` — Add throttling + 429 detection + +**New type:** + +```go +// RateLimitError is returned when the CoinGecko API responds with HTTP 429. +type RateLimitError struct { + Body string + RetryAfter time.Duration // cooldown duration; 0 means use a default +} + +func (e *RateLimitError) Error() string { + return fmt.Sprintf("rate limited: 429 %s", e.Body) +} +``` + +**New helper functions:** + +```go +// IsRateLimitError reports whether err is a *RateLimitError. +func IsRateLimitError(err error) bool { + var rle *RateLimitError + return errors.As(err, &rle) +} + +// RetryAfterFromError extracts the RetryAfter from a RateLimitError. +// Returns defaultDuration if err is not a RateLimitError or has RetryAfter == 0. +func RetryAfterFromError(err error, defaultDuration time.Duration) time.Duration { + var rle *RateLimitError + if errors.As(err, &rle) && rle.RetryAfter > 0 { + return rle.RetryAfter + } + return defaultDuration +} +``` + +**New constant:** + +```go +const DefaultRetryAfter = 60 * time.Second +``` + +**Changes to `HTTPClient`:** + +Add fields for throttling: + +```go +type HTTPClient struct { + httpClient *http.Client + baseURL string + apiKey string + mu sync.Mutex + lastRequestAt time.Time + minInterval time.Duration // minimum gap between API calls (default 2s) +} +``` + +- `NewHTTPClient` initializes `minInterval` to `2 * time.Second` and sets `lastRequestAt` to zero time. +- Add a `throttle(ctx context.Context) error` method that sleeps until `minInterval` has elapsed since `lastRequestAt`, then updates `lastRequestAt`. Respects context cancellation. +- Both `FetchMarkets` and `FetchPrices` call `c.throttle(ctx)` as the first action before building the request. + +**429 detection in `FetchMarkets` and `FetchPrices`:** + +Add an explicit 429 handler before the generic non-2xx handler: + +```go +if resp.StatusCode == http.StatusTooManyRequests { + body, _ := io.ReadAll(resp.Body) + rle := &RateLimitError{ + Body: string(body), + } + if retryAfterHeader := resp.Header.Get("Retry-After"); retryAfterHeader != "" { + if seconds, err := strconv.Atoi(retryAfterHeader); err == nil { + rle.RetryAfter = time.Duration(seconds) * time.Second + } + } + return nil, rle +} +``` + +**New unexported constructor for testing:** + +```go +// newHTTPClientWithInterval creates an HTTPClient with a custom minimum request interval. +// Used for testing; production code should use NewHTTPClient. +func newHTTPClientWithInterval(apiKey string, interval time.Duration, serverURL string) *HTTPClient { + // ... +} +``` + +### 2. `internal/api/coingecko_test.go` — Tests for throttling + 429 + +**New tests:** + +1. **`TestThrottleEnforcesMinimumInterval`** — Calls `FetchMarkets` twice in quick succession on an `httptest.NewServer`. Verify the second call takes at least `minInterval` to return. + +2. **`TestThrottleWithContextCancellation`** — Cancels context during throttle wait. Verify `FetchMarkets` returns context.Canceled error promptly. + +3. **`TestFetchMarketsRateLimit429`** — httptest server returns 429 with body `"rate limit exceeded"`. Verify that `FetchMarkets` returns a `*RateLimitError` with `Body` containing the response text. + +4. **`TestFetchPricesRateLimit429`** — Same as above for `FetchPrices`. + +5. **`TestFetchMarkets429WithRetryAfterHeader`** — httptest server returns 429 with `Retry-After: 30` header. Verify `RateLimitError.RetryAfter == 30s`. + +6. **`TestIsRateLimitError`** — Unit test for the helper with both `*RateLimitError` and non-rate-limit errors. + +7. **`TestRetryAfterFromError`** — Unit test: returns error's `RetryAfter` if set, else returns default. Also test with non-`RateLimitError`. + +8. **`TestNewHTTPClientDefaultMinInterval`** — Verify `NewHTTPClient("")` sets `minInterval` to 2 seconds. + +### 3. `internal/ui/markets.go` — Rate-limit state + status bar + +**New fields on `MarketsModel`:** + +```go +rateLimitedUntil time.Time // time at which rate-limit cooldown expires +refreshAttempts int // consecutive rate-limit errors (for backoff) +``` + +**Modified `errMsg` handler in `update()`:** + +```go +case errMsg: + m.refreshing = false + if api.IsRateLimitError(msg.err) { + backoff := api.RetryAfterFromError(msg.err, api.DefaultRetryAfter) + multiplier := 1 << min(m.refreshAttempts, 3) // 1, 2, 4, 8 (capped) + cooldown := backoff * time.Duration(multiplier) + if cooldown > 5*time.Minute { + cooldown = 5 * time.Minute + } + m.rateLimitedUntil = time.Now().Add(cooldown) + m.refreshAttempts++ + m.lastErr = "" // status bar shows rate-limit state, not raw error + } else { + m.lastErr = msg.err.Error() + } +``` + +**Modified `r` key handler:** + +```go +case 'r': + if m.refreshing || time.Now().Before(m.rateLimitedUntil) || len(m.coins) == 0 { + return m, nil // no-op: already refreshing, rate-limited, or no data + } + m.refreshing = true + return m, m.cmdRefresh() +``` + +**Modified `tickMsg` handler:** + +```go +case tickMsg: + cmds := []tea.Cmd{cmdTick()} + now := time.Now() + canRefresh := !m.refreshing && len(m.coins) > 0 && now.Sub(m.lastRefreshed) >= refreshInterval && now.After(m.rateLimitedUntil) + if canRefresh { + m.refreshing = true + cmds = append(cmds, m.cmdRefresh()) + } + return m, tea.Batch(cmds...) +``` + +**Modified `pricesUpdatedMsg` handler:** + +```go +case pricesUpdatedMsg: + m.coins = msg.coins + m.refreshing = false + m.lastErr = "" + m.lastRefreshed = time.Now() + m.refreshAttempts = 0 // reset backoff on success + m.rateLimitedUntil = time.Time{} // clear rate-limit state +``` + +**Modified `statusRight()`:** + +```go +func (m MarketsModel) statusRight() string { + now := time.Now() + if m.refreshing { + return "Refreshing" + } + if now.Before(m.rateLimitedUntil) { + secs := int(time.Until(m.rateLimitedUntil).Seconds()) + return fmt.Sprintf("Rate limited — retry in %ds", secs) + } + if m.lastErr != "" { + return "error: " + m.lastErr + } + if m.lastRefreshed.IsZero() { + return "loading..." + } + if now.Sub(m.lastRefreshed) > staleThreshold { + return "Stale" + } + return "Synced" +} +``` + +**Modified `renderStatusBar()`:** + +Change the status bar switch from exact string match to `strings.HasPrefix` for the rate-limit case: + +```go +var rightStyled string +switch { +case strings.HasPrefix(rightContent, "Rate limited"): + rightStyled = rateLimitStyle.Render(rightContent) +case rightContent == "Synced": + rightStyled = greenStyle.Render(rightContent) +case rightContent == "Stale": + rightStyled = yellowStyle.Render(rightContent) +case strings.HasPrefix(rightContent, "error:"): + rightStyled = errStyle.Render(rightContent) +default: + rightStyled = grayStyle.Render(rightContent) +} +``` + +New style constant in `renderStatusBar`: + +```go +rateLimitStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("#FF8C00")) // dark orange +``` + +### 4. `internal/ui/markets_test.go` — Tests for rate-limit state + +**New tests:** + +1. **`TestMarketsRateLimitedErrMsg`** — Send `errMsg{err: &api.RateLimitError{...}}` to model. Verify `rateLimitedUntil` is set to future time and `refreshAttempts` is 1. + +2. **`TestMarketsRateLimitedStatusBar`** — Set `rateLimitedUntil` to future time. Verify `statusRight()` returns `"Rate limited — retry in Xs"` with correct seconds. + +3. **`TestMarketsRateLimitedStatusBarNoLongerLimited`** — Set `rateLimitedUntil` to past time. Verify `statusRight()` returns normal state. + +4. **`TestMarketsRefreshBlockedWhenRateLimited`** — Set `rateLimitedUntil` to future time, send `r` key. Verify no refresh command returned and `refreshing` stays false. + +5. **`TestMarketsAutoRefreshBlockedWhenRateLimited`** — Set `rateLimitedUntil` to future time, send `tickMsg` with elapsed interval. Verify no refresh command. + +6. **`TestMarketsAutoRefreshResumesAfterCooldown`** — Set `rateLimitedUntil` to past time, send `tickMsg` with elapsed interval. Verify refresh fires. + +7. **`TestMarketsExponentialBackoffOnRepeated429`** — Send multiple `errMsg` with `RateLimitError`. Verify `rateLimitedUntil` increases exponentially (60s → 120s → 240s → 480s → capped at 300s). + +8. **`TestMarketsBackoffResetOnSuccess`** — Send `errMsg` with `RateLimitError`, then `pricesUpdatedMsg`. Verify `refreshAttempts` resets to 0 and `rateLimitedUntil` is zero time. + +9. **`TestMarketsNonRateLimitErrorDoesNotSetRateLimitedUntil`** — Send `errMsg{err: errors.New("network failed")}`. Verify `rateLimitedUntil` stays zero. + +10. **`TestMarketsRateLimitedStatusBarStyled`** — Verify the rendered status bar contains the rate-limit text. + +### 5. `internal/ui/testhelpers_test.go` — No structural changes needed + +`StubAPI.err` already supports returning arbitrary errors including `*api.RateLimitError`. + +## Implementation order + +1. Define `RateLimitError`, `IsRateLimitError`, `RetryAfterFromError`, and `DefaultRetryAfter` in `internal/api/coingecko.go` +2. Add throttle fields and `throttle()` method to `HTTPClient` +3. Add 429 detection in `FetchMarkets` and `FetchPrices` +4. Write tests for `RateLimitError`, throttle, and 429 detection in `internal/api/coingecko_test.go` +5. Add `rateLimitedUntil` and `refreshAttempts` fields to `MarketsModel` +6. Modify `errMsg` handler in `update()` to detect `RateLimitError` and set backoff +7. Modify `r` key handler to no-op when rate-limited +8. Modify `tickMsg` handler to respect rate-limit cooldown +9. Modify `pricesUpdatedMsg` handler to reset backoff state +10. Modify `statusRight()` to show rate-limited countdown +11. Modify `renderStatusBar()` to style the rate-limited state +12. Write tests for all MarketsModel rate-limit state transitions in `internal/ui/markets_test.go` +13. Run `make check` and fix any issues + +## Verification + +```bash +make fmt # gofumpt formatting +make lint # golangci-lint +make test # go test -race -coverprofile=coverage.out ./... +make vuln # govulncheck +make check # all of the above +``` + +All tests must pass non-interactively with no network access. The throttle tests use short intervals (e.g. 10ms) to keep test execution fast. + +## Branch + +`feat/012-coingecko-rate-limiting` \ No newline at end of file diff --git a/docs/reviews/012-coingecko-rate-limiting/revision-1.md b/docs/reviews/012-coingecko-rate-limiting/revision-1.md new file mode 100644 index 0000000..8541254 --- /dev/null +++ b/docs/reviews/012-coingecko-rate-limiting/revision-1.md @@ -0,0 +1,25 @@ +--- +branch: feat/012-coingecko-rate-limiting +revision: 1 +status: done +--- + +# Slice 012 — CoinGecko Rate Limiting (Revision 1) + +## Smoke test + completeness audit + +No findings. All scope items implemented, test coverage adequate, verification +commands satisfied. + +## Implementation review + +| ID | Sev | Status | Summary | +|----|-----|--------|---------| +| I1 | LOW | FIXED | Throttle has theoretical race between unlock and timer fire | +| I2 | LOW | FIXED | Inconsistent `io.ReadAll` error handling in 429 path | + +**I1** `internal/api/coingecko.go:92-117` +The `throttle()` method releases `mu` before sleeping on the timer, then re-acquires it to update `lastRequestAt`. If two goroutines call `throttle()` concurrently, both could calculate short sleep durations based on the same stale `lastRequestAt`, wake at nearly the same time, and both update `lastRequestAt` — violating the minimum interval guarantee. This is mitigated in practice by the UI-level `refreshing` flag that serialises API calls, so concurrent throttle calls won't occur in the current app. If the `HTTPClient` is ever used concurrently (e.g., from multiple tabs), the throttle could allow requests closer together than `minInterval`. Consider holding the lock through the sleep (using a condition variable) or recording the intended wake time atomically before releasing the lock. + +**I2** `internal/api/coingecko.go:163` and `internal/api/coingecko.go:241` +In the 429 handler, `io.ReadAll(resp.Body)` errors are ignored (`body, _ := io.ReadAll(resp.Body)`), while the generic non-2xx handler at lines 176 and 254 properly checks `readErr`. Slice 11 explicitly fixed ignored `io.ReadAll` errors in those other paths. For a 429 response, the body is informational (used in the error message), so ignoring the read error is low-risk — the `RateLimitError` is still returned with status 429. However, it's inconsistent with the codebase convention established in Slice 11. Consider handling the error the same way: `body, readErr := io.ReadAll(resp.Body)` and incorporating `readErr` into the `RateLimitError.Body` or `Error()` string if non-nil. \ No newline at end of file diff --git a/docs/roadmap.md b/docs/roadmap.md index fc9b3a4..f2ba626 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -118,7 +118,7 @@ Small, targeted code changes with no new runtime behaviour. - **TDD:** no new tests required; verify existing suite stays green ## Slice 12 — CoinGecko rate limiting -STATUS: PENDING +STATUS: DONE New runtime behaviour: the app must stay within the free-tier limit of ~30 req/min. diff --git a/internal/api/coingecko.go b/internal/api/coingecko.go index a6c5efd..70ea1a9 100644 --- a/internal/api/coingecko.go +++ b/internal/api/coingecko.go @@ -3,17 +3,48 @@ package api import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" "net/url" "strconv" "strings" + "sync" "time" "github.com/fredericomozzato/crypto_tracker/internal/store" ) +// DefaultRetryAfter is the default cooldown duration when rate limited. +const DefaultRetryAfter = 60 * time.Second + +// RateLimitError is returned when the CoinGecko API responds with HTTP 429. +type RateLimitError struct { + Body string + RetryAfter time.Duration // cooldown duration; 0 means use a default +} + +func (e *RateLimitError) Error() string { + return fmt.Sprintf("rate limited: 429 %s", e.Body) +} + +// IsRateLimitError reports whether err is a *RateLimitError. +func IsRateLimitError(err error) bool { + var rle *RateLimitError + return errors.As(err, &rle) +} + +// RetryAfterFromError extracts the RetryAfter from a RateLimitError. +// Returns defaultDuration if err is not a RateLimitError or has RetryAfter == 0. +func RetryAfterFromError(err error, defaultDuration time.Duration) time.Duration { + var rle *RateLimitError + if errors.As(err, &rle) && rle.RetryAfter > 0 { + return rle.RetryAfter + } + return defaultDuration +} + // CoinGeckoClient defines the interface for fetching cryptocurrency market data. type CoinGeckoClient interface { FetchMarkets(ctx context.Context, limit int) ([]store.Coin, error) @@ -22,9 +53,12 @@ type CoinGeckoClient interface { // HTTPClient implements CoinGeckoClient using HTTP requests. type HTTPClient struct { - httpClient *http.Client - baseURL string - apiKey string + httpClient *http.Client + baseURL string + apiKey string + mu sync.Mutex + lastRequestAt time.Time + minInterval time.Duration // minimum gap between API calls (default 2s) } // NewHTTPClient creates a new HTTP client for CoinGecko API. @@ -37,10 +71,49 @@ func NewHTTPClient(apiKey string) *HTTPClient { } return &HTTPClient{ - httpClient: &http.Client{Timeout: 15 * time.Second}, - baseURL: baseURL, - apiKey: apiKey, + httpClient: &http.Client{Timeout: 15 * time.Second}, + baseURL: baseURL, + apiKey: apiKey, + minInterval: 2 * time.Second, + } +} + +// newHTTPClientWithInterval creates an HTTPClient with a custom minimum request interval. +// Used for testing; production code should use NewHTTPClient. +func newHTTPClientWithInterval(apiKey string, interval time.Duration, serverURL string) *HTTPClient { + return &HTTPClient{ + httpClient: &http.Client{Timeout: 15 * time.Second}, + baseURL: serverURL, + apiKey: apiKey, + minInterval: interval, + } +} + +// throttle sleeps until minInterval has elapsed since lastRequestAt. +// Respects context cancellation. +func (c *HTTPClient) throttle(ctx context.Context) error { + c.mu.Lock() + elapsed := time.Since(c.lastRequestAt) + if elapsed < c.minInterval { + sleepDuration := c.minInterval - elapsed + // Record the intended wake time before releasing lock to prevent + // concurrent callers from calculating stale sleep durations. + c.lastRequestAt = time.Now().Add(sleepDuration) + c.mu.Unlock() + + timer := time.NewTimer(sleepDuration) + defer timer.Stop() + + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } } + c.lastRequestAt = time.Now() + c.mu.Unlock() + return nil } // coinGeckoMarketResponse represents a single item from the /coins/markets endpoint. @@ -55,6 +128,10 @@ type coinGeckoMarketResponse struct { // FetchMarkets fetches market data for the top cryptocurrencies. func (c *HTTPClient) FetchMarkets(ctx context.Context, limit int) ([]store.Coin, error) { + if err := c.throttle(ctx); err != nil { + return nil, err + } + params := url.Values{} params.Set("vs_currency", "usd") params.Set("order", "market_cap_desc") @@ -82,6 +159,22 @@ func (c *HTTPClient) FetchMarkets(ctx context.Context, limit int) ([]store.Coin, _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusTooManyRequests { + body, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return nil, fmt.Errorf("fetching markets: %d (failed to read response body: %w)", resp.StatusCode, readErr) + } + rle := &RateLimitError{ + Body: string(body), + } + if retryAfterHeader := resp.Header.Get("Retry-After"); retryAfterHeader != "" { + if seconds, err := strconv.Atoi(retryAfterHeader); err == nil { + rle.RetryAfter = time.Duration(seconds) * time.Second + } + } + return nil, rle + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { body, readErr := io.ReadAll(resp.Body) if readErr != nil { @@ -116,6 +209,10 @@ type coinGeckoPriceResponse map[string]map[string]float64 // FetchPrices fetches current prices for the given coin API IDs. // Returns a map of api_id -> USD price. func (c *HTTPClient) FetchPrices(ctx context.Context, apiIDs []string) (map[string]float64, error) { + if err := c.throttle(ctx); err != nil { + return nil, err + } + if len(apiIDs) == 0 { return make(map[string]float64), nil } @@ -143,6 +240,22 @@ func (c *HTTPClient) FetchPrices(ctx context.Context, apiIDs []string) (map[stri _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusTooManyRequests { + body, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return nil, fmt.Errorf("fetching prices: %d (failed to read response body: %w)", resp.StatusCode, readErr) + } + rle := &RateLimitError{ + Body: string(body), + } + if retryAfterHeader := resp.Header.Get("Retry-After"); retryAfterHeader != "" { + if seconds, err := strconv.Atoi(retryAfterHeader); err == nil { + rle.RetryAfter = time.Duration(seconds) * time.Second + } + } + return nil, rle + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { body, readErr := io.ReadAll(resp.Body) if readErr != nil { diff --git a/internal/api/coingecko_test.go b/internal/api/coingecko_test.go index 67b9a60..5e60e43 100644 --- a/internal/api/coingecko_test.go +++ b/internal/api/coingecko_test.go @@ -3,10 +3,13 @@ package api import ( "context" "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" "strings" "testing" + "time" ) func TestFetchMarketsSuccess(t *testing.T) { @@ -332,3 +335,249 @@ func TestFetchPricesWithAPIKey(t *testing.T) { t.Errorf("expected bitcoin price 67000.00, got %f", prices["bitcoin"]) } } + +// Rate limiting tests + +func TestThrottleEnforcesMinimumInterval(t *testing.T) { + requestCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + response := []map[string]interface{}{ + { + "id": "bitcoin", + "symbol": "btc", + "name": "Bitcoin", + "current_price": 67000.00, + }, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(response) + })) + defer server.Close() + + minInterval := 50 * time.Millisecond + client := newHTTPClientWithInterval("", minInterval, server.URL) + + // First request + start := time.Now() + _, err := client.FetchMarkets(context.Background(), 1) + if err != nil { + t.Fatalf("unexpected error on first call: %v", err) + } + + // Second request should be throttled + _, err = client.FetchMarkets(context.Background(), 1) + elapsed := time.Since(start) + if err != nil { + t.Fatalf("unexpected error on second call: %v", err) + } + + if elapsed < minInterval { + t.Errorf("expected second call to be throttled for at least %v, but took %v", minInterval, elapsed) + } + + if requestCount != 2 { + t.Errorf("expected 2 requests, got %d", requestCount) + } +} + +func TestThrottleWithContextCancellation(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + response := []map[string]interface{}{} + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(response) + })) + defer server.Close() + + minInterval := 100 * time.Millisecond + client := newHTTPClientWithInterval("", minInterval, server.URL) + + // First request to set lastRequestAt + _, _ = client.FetchMarkets(context.Background(), 1) + + // Second request with cancelled context + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(10 * time.Millisecond) + cancel() + }() + + _, err := client.FetchMarkets(ctx, 1) + if err == nil { + t.Fatal("expected error for cancelled context, got nil") + } + + if !strings.Contains(err.Error(), "context canceled") { + t.Errorf("expected context canceled error, got: %s", err.Error()) + } +} + +func TestFetchMarketsRateLimit429(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte("rate limit exceeded")) + })) + defer server.Close() + + client := newHTTPClientWithInterval("", 10*time.Millisecond, server.URL) + + _, err := client.FetchMarkets(context.Background(), 1) + if err == nil { + t.Fatal("expected error, got nil") + } + + if !IsRateLimitError(err) { + t.Errorf("expected IsRateLimitError to return true, got false") + } + + var rle *RateLimitError + if !errors.As(err, &rle) { + t.Fatalf("expected error to be *RateLimitError, got %T", err) + } + + if !strings.Contains(rle.Body, "rate limit exceeded") { + t.Errorf("expected Body to contain 'rate limit exceeded', got %q", rle.Body) + } +} + +func TestFetchPricesRateLimit429(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte("API rate limit exceeded")) + })) + defer server.Close() + + client := newHTTPClientWithInterval("", 10*time.Millisecond, server.URL) + + _, err := client.FetchPrices(context.Background(), []string{"bitcoin"}) + if err == nil { + t.Fatal("expected error, got nil") + } + + if !IsRateLimitError(err) { + t.Errorf("expected IsRateLimitError to return true, got false") + } + + var rle *RateLimitError + if !errors.As(err, &rle) { + t.Fatalf("expected error to be *RateLimitError, got %T", err) + } + + if !strings.Contains(rle.Body, "API rate limit exceeded") { + t.Errorf("expected Body to contain 'API rate limit exceeded', got %q", rle.Body) + } +} + +func TestFetchMarkets429WithRetryAfterHeader(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "30") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte("rate limit exceeded")) + })) + defer server.Close() + + client := newHTTPClientWithInterval("", 10*time.Millisecond, server.URL) + + _, err := client.FetchMarkets(context.Background(), 1) + if err == nil { + t.Fatal("expected error, got nil") + } + + var rle *RateLimitError + if !errors.As(err, &rle) { + t.Fatalf("expected error to be *RateLimitError, got %T", err) + } + + if rle.RetryAfter != 30*time.Second { + t.Errorf("expected RetryAfter 30s, got %v", rle.RetryAfter) + } +} + +func TestIsRateLimitError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "RateLimitError returns true", + err: &RateLimitError{Body: "rate limited"}, + expected: true, + }, + { + name: "Regular error returns false", + err: errors.New("network error"), + expected: false, + }, + { + name: "Nil error returns false", + err: nil, + expected: false, + }, + { + name: "Wrapped RateLimitError returns true", + err: fmt.Errorf("wrapped: %w", &RateLimitError{Body: "rate limited"}), + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsRateLimitError(tt.err) + if got != tt.expected { + t.Errorf("IsRateLimitError() = %v, want %v", got, tt.expected) + } + }) + } +} + +func TestRetryAfterFromError(t *testing.T) { + tests := []struct { + name string + err error + defaultDuration time.Duration + expected time.Duration + }{ + { + name: "RateLimitError with RetryAfter set", + err: &RateLimitError{Body: "rate limited", RetryAfter: 30 * time.Second}, + defaultDuration: 60 * time.Second, + expected: 30 * time.Second, + }, + { + name: "RateLimitError with zero RetryAfter uses default", + err: &RateLimitError{Body: "rate limited"}, + defaultDuration: 60 * time.Second, + expected: 60 * time.Second, + }, + { + name: "Non-RateLimitError returns default", + err: errors.New("network error"), + defaultDuration: 60 * time.Second, + expected: 60 * time.Second, + }, + { + name: "Nil error returns default", + err: nil, + defaultDuration: 60 * time.Second, + expected: 60 * time.Second, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := RetryAfterFromError(tt.err, tt.defaultDuration) + if got != tt.expected { + t.Errorf("RetryAfterFromError() = %v, want %v", got, tt.expected) + } + }) + } +} + +func TestNewHTTPClientDefaultMinInterval(t *testing.T) { + client := NewHTTPClient("") + + if client.minInterval != 2*time.Second { + t.Errorf("expected minInterval to be 2s, got %v", client.minInterval) + } +} diff --git a/internal/ui/markets.go b/internal/ui/markets.go index ec0cd9f..c5ce049 100644 --- a/internal/ui/markets.go +++ b/internal/ui/markets.go @@ -16,17 +16,19 @@ import ( // MarketsModel manages the Markets tab: coin list, auto-refresh, cursor, status bar. type MarketsModel struct { - width int - height int - ctx context.Context - store store.Store - client api.CoinGeckoClient - coins []store.Coin - lastErr string - refreshing bool - lastRefreshed time.Time - cursor int - offset int + width int + height int + ctx context.Context + store store.Store + client api.CoinGeckoClient + coins []store.Coin + lastErr string + refreshing bool + lastRefreshed time.Time + cursor int + offset int + rateLimitedUntil time.Time // time at which rate-limit cooldown expires + refreshAttempts int // consecutive rate-limit errors (for backoff) } // coinsLoadedMsg is sent when coins are successfully loaded from the API. @@ -112,7 +114,7 @@ func (m MarketsModel) update(msg tea.Msg) (MarketsModel, tea.Cmd) { for _, r := range msg.Runes { switch r { case 'r': - if !m.refreshing && len(m.coins) > 0 { + if !m.refreshing && time.Now().After(m.rateLimitedUntil) && len(m.coins) > 0 { m.refreshing = true return m, m.cmdRefresh() } @@ -140,7 +142,9 @@ func (m MarketsModel) update(msg tea.Msg) (MarketsModel, tea.Cmd) { m.height = msg.Height case tickMsg: cmds := []tea.Cmd{cmdTick()} - if !m.refreshing && len(m.coins) > 0 && time.Since(m.lastRefreshed) >= refreshInterval { + now := time.Now() + canRefresh := !m.refreshing && len(m.coins) > 0 && now.Sub(m.lastRefreshed) >= refreshInterval && now.After(m.rateLimitedUntil) + if canRefresh { m.refreshing = true cmds = append(cmds, m.cmdRefresh()) } @@ -160,9 +164,23 @@ func (m MarketsModel) update(msg tea.Msg) (MarketsModel, tea.Cmd) { m.refreshing = false m.lastErr = "" m.lastRefreshed = time.Now() + m.refreshAttempts = 0 // reset backoff on success + m.rateLimitedUntil = time.Time{} // clear rate-limit state case errMsg: - m.lastErr = msg.err.Error() m.refreshing = false + if api.IsRateLimitError(msg.err) { + backoff := api.RetryAfterFromError(msg.err, api.DefaultRetryAfter) + multiplier := 1 << min(m.refreshAttempts, 3) // 1, 2, 4, 8 (capped) + cooldown := backoff * time.Duration(multiplier) + if cooldown > 5*time.Minute { + cooldown = 5 * time.Minute + } + m.rateLimitedUntil = time.Now().Add(cooldown) + m.refreshAttempts++ + m.lastErr = "" // status bar shows rate-limit state, not raw error + } else { + m.lastErr = msg.err.Error() + } } return m, nil @@ -269,16 +287,21 @@ func (m MarketsModel) View() string { // statusRight returns the right-hand portion of the status bar. func (m MarketsModel) statusRight() string { + now := time.Now() if m.refreshing { return "Refreshing" } + if now.Before(m.rateLimitedUntil) { + secs := int(time.Until(m.rateLimitedUntil).Seconds()) + return fmt.Sprintf("Rate limited — retry in %ds", secs) + } if m.lastErr != "" { return "error: " + m.lastErr } if m.lastRefreshed.IsZero() { return "loading..." } - if time.Since(m.lastRefreshed) > staleThreshold { + if now.Sub(m.lastRefreshed) > staleThreshold { return "Stale" } return "Synced" @@ -295,13 +318,17 @@ func (m MarketsModel) renderStatusBar() string { greenStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("#00FF00")) yellowStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("#FFD700")) + rateLimitStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("#FF8C00")) // dark orange + var rightStyled string - switch rightContent { - case "Synced": + switch { + case strings.HasPrefix(rightContent, "Rate limited"): + rightStyled = rateLimitStyle.Render(rightContent) + case rightContent == "Synced": rightStyled = greenStyle.Render(rightContent) - case "Stale": + case rightContent == "Stale": rightStyled = yellowStyle.Render(rightContent) - case "error: " + m.lastErr: + case strings.HasPrefix(rightContent, "error:"): rightStyled = errStyle.Render(rightContent) default: rightStyled = grayStyle.Render(rightContent) diff --git a/internal/ui/markets_test.go b/internal/ui/markets_test.go index 9071a37..64ee4bc 100644 --- a/internal/ui/markets_test.go +++ b/internal/ui/markets_test.go @@ -7,13 +7,14 @@ import ( "time" tea "github.com/charmbracelet/bubbletea" + "github.com/fredericomozzato/crypto_tracker/internal/api" "github.com/fredericomozzato/crypto_tracker/internal/store" ) func TestMarketsInitReturnsBatchedCmd(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) cmd := m.Init() if cmd == nil { @@ -23,8 +24,8 @@ func TestMarketsInitReturnsBatchedCmd(t *testing.T) { func TestMarketsCoinsLoadedMsg(t *testing.T) { s := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, s, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, s, apiStub) m.width = 100 m.height = 30 @@ -75,8 +76,8 @@ func TestMarketsCoinsLoadedMsg(t *testing.T) { func TestMarketsErrMsg(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -96,8 +97,8 @@ func TestMarketsErrMsg(t *testing.T) { func TestMarketsViewRendersLoading(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -141,8 +142,8 @@ func TestMarketsViewRendersHintLine(t *testing.T) { func TestMarketsRefreshKeyReturnsCmdWhenCoinsLoaded(t *testing.T) { storeStub := &StubStore{coins: []store.Coin{{ApiID: "bitcoin", Name: "Bitcoin", Ticker: "BTC", Rate: 67000.00}}} - api := &StubAPI{prices: map[string]float64{"bitcoin": 68000.00}} - m := NewMarketsModel(testCtx, storeStub, api) + apiStub := &StubAPI{prices: map[string]float64{"bitcoin": 68000.00}} + m := NewMarketsModel(testCtx, storeStub, apiStub) m.width = 100 m.height = 30 @@ -163,8 +164,8 @@ func TestMarketsRefreshKeyReturnsCmdWhenCoinsLoaded(t *testing.T) { func TestMarketsRefreshKeyIgnoredWhenAlreadyRefreshing(t *testing.T) { stub := &StubStore{coins: []store.Coin{{ApiID: "bitcoin", Name: "Bitcoin", Ticker: "BTC", Rate: 67000.00}}} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = stub.coins @@ -180,8 +181,8 @@ func TestMarketsRefreshKeyIgnoredWhenAlreadyRefreshing(t *testing.T) { func TestMarketsRefreshKeyIgnoredWhenNoCoins(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -195,8 +196,8 @@ func TestMarketsRefreshKeyIgnoredWhenNoCoins(t *testing.T) { func TestMarketsPricesUpdatedMsg(t *testing.T) { storeStub := &StubStore{coins: []store.Coin{{ApiID: "bitcoin", Name: "Bitcoin", Ticker: "BTC", Rate: 68000.00}}} - api := &StubAPI{} - m := NewMarketsModel(testCtx, storeStub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, storeStub, apiStub) m.width = 100 m.height = 30 m.coins = []store.Coin{{ApiID: "bitcoin", Name: "Bitcoin", Ticker: "BTC", Rate: 67000.00}} @@ -216,8 +217,8 @@ func TestMarketsPricesUpdatedMsg(t *testing.T) { func TestMarketsViewShowsRefreshHint(t *testing.T) { stub := &StubStore{coins: []store.Coin{{ApiID: "bitcoin", Name: "Bitcoin", Ticker: "BTC", Rate: 67000.00, MarketRank: 1}}} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = stub.coins @@ -298,8 +299,8 @@ func TestMarketsCursorMovesUpOnUpArrow(t *testing.T) { func TestMarketsMoveCursorNoPanicOnEmptyCoins(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -316,8 +317,8 @@ func TestMarketsMoveCursorNoPanicOnEmptyCoins(t *testing.T) { func TestMarketsCursorClampedAfterCoinsLoaded(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -330,8 +331,8 @@ func TestMarketsCursorClampedAfterCoinsLoaded(t *testing.T) { func TestMarketsTickMsgAlwaysReissuesTicker(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) updated, cmd := m.update(tickMsg(time.Now())) @@ -343,8 +344,8 @@ func TestMarketsTickMsgAlwaysReissuesTicker(t *testing.T) { func TestMarketsTickMsgBelow60sDoesNotRefresh(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.coins = threeCoins() m.lastRefreshed = time.Now().Add(-30 * time.Second) @@ -357,8 +358,8 @@ func TestMarketsTickMsgBelow60sDoesNotRefresh(t *testing.T) { func TestMarketsTickMsgAbove60sFiresRefresh(t *testing.T) { stub := &StubStore{coins: threeCoins()} - api := &StubAPI{prices: map[string]float64{"coin-1": 100.0}} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{prices: map[string]float64{"coin-1": 100.0}} + m := NewMarketsModel(testCtx, stub, apiStub) m.coins = threeCoins() m.lastRefreshed = time.Now().Add(-61 * time.Second) m.refreshing = false @@ -375,8 +376,8 @@ func TestMarketsTickMsgAbove60sFiresRefresh(t *testing.T) { func TestMarketsTickMsgWhenAlreadyRefreshing(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.coins = threeCoins() m.lastRefreshed = time.Now().Add(-61 * time.Second) m.refreshing = true @@ -393,8 +394,8 @@ func TestMarketsTickMsgWhenAlreadyRefreshing(t *testing.T) { func TestMarketsTickMsgWhenNoCoins(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.lastRefreshed = time.Now().Add(-61 * time.Second) updated, _ := m.update(tickMsg(time.Now())) @@ -406,8 +407,8 @@ func TestMarketsTickMsgWhenNoCoins(t *testing.T) { func TestMarketsCoinsLoadedSetsLastRefreshed(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -424,8 +425,8 @@ func TestMarketsCoinsLoadedSetsLastRefreshed(t *testing.T) { func TestMarketsPricesUpdatedSetsLastRefreshed(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -443,8 +444,8 @@ func TestMarketsPricesUpdatedSetsLastRefreshed(t *testing.T) { func TestMarketsStatusBarShowsLoading(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 @@ -456,8 +457,8 @@ func TestMarketsStatusBarShowsLoading(t *testing.T) { func TestMarketsStatusBarShowsRefreshing(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -472,8 +473,8 @@ func TestMarketsStatusBarShowsRefreshing(t *testing.T) { func TestMarketsStatusBarShowsError(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -488,8 +489,8 @@ func TestMarketsStatusBarShowsError(t *testing.T) { func TestMarketsStatusBarShowsSynced(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -503,8 +504,8 @@ func TestMarketsStatusBarShowsSynced(t *testing.T) { func TestMarketsStatusBarShowsStale(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -518,8 +519,8 @@ func TestMarketsStatusBarShowsStale(t *testing.T) { func TestMarketsTableRendersWhileError(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -537,8 +538,8 @@ func TestMarketsTableRendersWhileError(t *testing.T) { func TestMarketsStatusBarHasHintsOnLeft(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) m.width = 100 m.height = 30 m.coins = threeCoins() @@ -552,9 +553,9 @@ func TestMarketsStatusBarHasHintsOnLeft(t *testing.T) { func TestMarketsInitFetchesHundredCoinsOnFirstLaunch(t *testing.T) { coins := threeCoins() - api := &StubAPI{coins: coins} + apiStub := &StubAPI{coins: coins} s := &StubStore{} - m := NewMarketsModel(testCtx, s, api) + m := NewMarketsModel(testCtx, s, apiStub) msg := executeInitBatchForMarkets(t, m) @@ -565,16 +566,16 @@ func TestMarketsInitFetchesHundredCoinsOnFirstLaunch(t *testing.T) { if len(loaded.coins) != 3 { t.Errorf("expected 3 coins, got %d", len(loaded.coins)) } - if len(api.fetchMarketsCalls) != 1 || api.fetchMarketsCalls[0] != 100 { - t.Errorf("expected FetchMarkets called with 100, got %v", api.fetchMarketsCalls) + if len(apiStub.fetchMarketsCalls) != 1 || apiStub.fetchMarketsCalls[0] != 100 { + t.Errorf("expected FetchMarkets called with 100, got %v", apiStub.fetchMarketsCalls) } } func TestMarketsInitLoadsFromDBOnSubsequentLaunch(t *testing.T) { coins := makeCoins(100) - api := &StubAPI{coins: coins} + apiStub := &StubAPI{coins: coins} s := &StubStore{coins: coins} - m := NewMarketsModel(testCtx, s, api) + m := NewMarketsModel(testCtx, s, apiStub) msg := executeInitBatchForMarkets(t, m) @@ -585,17 +586,17 @@ func TestMarketsInitLoadsFromDBOnSubsequentLaunch(t *testing.T) { if len(loaded.coins) != 100 { t.Errorf("expected 100 coins from DB, got %d", len(loaded.coins)) } - if len(api.fetchMarketsCalls) != 0 { - t.Errorf("expected no API calls, got %v", api.fetchMarketsCalls) + if len(apiStub.fetchMarketsCalls) != 0 { + t.Errorf("expected no API calls, got %v", apiStub.fetchMarketsCalls) } } func TestMarketsInitRefetchesWhenDBPartiallySeeded(t *testing.T) { partial := makeCoins(10) full := makeCoins(100) - api := &StubAPI{coins: full} + apiStub := &StubAPI{coins: full} s := &StubStore{coins: partial} - m := NewMarketsModel(testCtx, s, api) + m := NewMarketsModel(testCtx, s, apiStub) msg := executeInitBatchForMarkets(t, m) @@ -606,15 +607,15 @@ func TestMarketsInitRefetchesWhenDBPartiallySeeded(t *testing.T) { if len(loaded.coins) != 100 { t.Errorf("expected 100 coins after refetch, got %d", len(loaded.coins)) } - if len(api.fetchMarketsCalls) != 1 || api.fetchMarketsCalls[0] != 100 { - t.Errorf("expected FetchMarkets called with 100, got %v", api.fetchMarketsCalls) + if len(apiStub.fetchMarketsCalls) != 1 || apiStub.fetchMarketsCalls[0] != 100 { + t.Errorf("expected FetchMarkets called with 100, got %v", apiStub.fetchMarketsCalls) } } func TestMarketsIgnoresOtherKeys(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) otherKeys := []rune{'a', 'b', 'c', 'x', 'z', '1', '2', ' '} for _, key := range otherKeys { msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}} @@ -627,8 +628,8 @@ func TestMarketsIgnoresOtherKeys(t *testing.T) { func TestMarketsInputActiveFalse(t *testing.T) { stub := &StubStore{} - api := &StubAPI{} - m := NewMarketsModel(testCtx, stub, api) + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) if m.InputActive() { t.Error("expected InputActive() to return false for MarketsModel") } @@ -652,3 +653,231 @@ func executeInitBatchForMarkets(t *testing.T, m MarketsModel) tea.Msg { t.Fatal("no coinsLoadedMsg or errMsg in batch") return nil } + +// Rate limiting tests + +func TestMarketsRateLimitedErrMsg(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + + rle := &api.RateLimitError{Body: "rate limit exceeded"} + msg := errMsg{err: rle} + updated, _ := m.update(msg) + + if updated.refreshAttempts != 1 { + t.Errorf("expected refreshAttempts 1, got %d", updated.refreshAttempts) + } + + if updated.rateLimitedUntil.IsZero() { + t.Error("expected rateLimitedUntil to be set") + } + + if time.Until(updated.rateLimitedUntil) <= 0 { + t.Error("expected rateLimitedUntil to be in the future") + } +} + +func TestMarketsRateLimitedStatusBar(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + m.rateLimitedUntil = time.Now().Add(30 * time.Second) + + right := m.statusRight() + if !strings.HasPrefix(right, "Rate limited") { + t.Errorf("expected statusRight to start with 'Rate limited', got %q", right) + } + + if !strings.Contains(right, "30s") && !strings.Contains(right, "29s") && !strings.Contains(right, "28s") { + t.Errorf("expected statusRight to contain retry seconds, got %q", right) + } +} + +func TestMarketsRateLimitedStatusBarNoLongerLimited(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + m.lastRefreshed = time.Now() + m.rateLimitedUntil = time.Now().Add(-30 * time.Second) // In the past + + right := m.statusRight() + if right != "Synced" { + t.Errorf("expected statusRight 'Synced', got %q", right) + } +} + +func TestMarketsRefreshBlockedWhenRateLimited(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + m.rateLimitedUntil = time.Now().Add(30 * time.Second) + m.refreshing = false + + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'r'}} + updated, cmd := m.update(msg) + + if cmd != nil { + t.Error("expected nil cmd when rate-limited") + } + + if updated.refreshing { + t.Error("expected refreshing to stay false") + } +} + +func TestMarketsAutoRefreshBlockedWhenRateLimited(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.coins = threeCoins() + m.lastRefreshed = time.Now().Add(-61 * time.Second) + m.refreshing = false + m.rateLimitedUntil = time.Now().Add(30 * time.Second) + + updated, cmd := m.update(tickMsg(time.Now())) + + if updated.refreshing { + t.Error("expected refreshing to stay false when rate-limited") + } + + // cmdTick is always returned, but no refresh cmd should be in the batch + if cmd == nil { + t.Error("expected non-nil cmd from tickMsg (ticker should be re-armed)") + } +} + +func TestMarketsAutoRefreshResumesAfterCooldown(t *testing.T) { + stub := &StubStore{coins: threeCoins()} + apiStub := &StubAPI{prices: map[string]float64{"coin-1": 100.0}} + m := NewMarketsModel(testCtx, stub, apiStub) + m.coins = threeCoins() + m.lastRefreshed = time.Now().Add(-61 * time.Second) + m.refreshing = false + m.rateLimitedUntil = time.Now().Add(-1 * time.Second) // Expired + + updated, cmd := m.update(tickMsg(time.Now())) + + if !updated.refreshing { + t.Error("expected refreshing to be true when cooldown expired") + } + + if cmd == nil { + t.Error("expected non-nil cmd when refresh fires") + } +} + +func TestMarketsExponentialBackoffOnRepeated429(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + + rle := &api.RateLimitError{Body: "rate limit exceeded"} + + for i := 0; i < 5; i++ { + beforeUpdate := time.Now() + updated, _ := m.update(errMsg{err: rle}) + + cooldown := updated.rateLimitedUntil.Sub(beforeUpdate) + + switch i { + case 0: + if cooldown < 59*time.Second || cooldown > 61*time.Second { + t.Errorf("attempt 1: expected cooldown ~60s, got %v", cooldown) + } + case 1: + if cooldown < 119*time.Second || cooldown > 121*time.Second { + t.Errorf("attempt 2: expected cooldown ~120s, got %v", cooldown) + } + case 2: + if cooldown < 239*time.Second || cooldown > 241*time.Second { + t.Errorf("attempt 3: expected cooldown ~240s, got %v", cooldown) + } + case 3: + if cooldown < 299*time.Second || cooldown > 301*time.Second { + t.Errorf("attempt 4: expected cooldown capped at ~300s, got %v", cooldown) + } + case 4: + // Should still be capped at 300s + if cooldown < 299*time.Second || cooldown > 301*time.Second { + t.Errorf("attempt 5: expected cooldown capped at ~300s, got %v", cooldown) + } + } + + // Update m for next iteration, simulating time passing + m = updated + m.rateLimitedUntil = time.Now() // Reset so next error triggers new calculation + } +} + +func TestMarketsBackoffResetOnSuccess(t *testing.T) { + stub := &StubStore{coins: threeCoins()} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + m.refreshing = true + m.refreshAttempts = 3 + m.rateLimitedUntil = time.Now().Add(30 * time.Second) + + updated, _ := m.update(pricesUpdatedMsg{coins: threeCoins()}) + + if updated.refreshAttempts != 0 { + t.Errorf("expected refreshAttempts reset to 0, got %d", updated.refreshAttempts) + } + + if !updated.rateLimitedUntil.IsZero() { + t.Errorf("expected rateLimitedUntil to be zero time, got %v", updated.rateLimitedUntil) + } +} + +func TestMarketsNonRateLimitErrorDoesNotSetRateLimitedUntil(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + + msg := errMsg{err: errors.New("network failed")} + updated, _ := m.update(msg) + + if !updated.rateLimitedUntil.IsZero() { + t.Errorf("expected rateLimitedUntil to remain zero, got %v", updated.rateLimitedUntil) + } + + if updated.lastErr != "network failed" { + t.Errorf("expected lastErr 'network failed', got %q", updated.lastErr) + } +} + +func TestMarketsRateLimitedStatusBarStyled(t *testing.T) { + stub := &StubStore{} + apiStub := &StubAPI{} + m := NewMarketsModel(testCtx, stub, apiStub) + m.width = 100 + m.height = 30 + m.coins = threeCoins() + m.rateLimitedUntil = time.Now().Add(30 * time.Second) + + view := m.View() + if !strings.Contains(view, "Rate limited") { + t.Errorf("expected view to contain 'Rate limited', got %q", view) + } +}