Skip to content

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Oct 15, 2025

Add retries and only alert after 3 failed attempts to reduce the alert noise

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability of APR data retrieval with automatic retries and delays on transient failures.
    • Enhanced error reporting and metrics after repeated failures to aid diagnostics.
    • Users should experience fewer intermittent gaps or stale APR values due to network hiccups.
    • No changes required to user settings or workflows; behavior is more resilient behind the scenes.

Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds retry logic to the Numia APR fetcher with configurable retry count and delay. Errors are logged per attempt, with sleep between retries. On success, results are returned immediately; on exhaustion, an error is returned and a counter is incremented. Introduces new constants and imports for fmt and time.

Changes

Cohort / File(s) Summary
Retry logic for Numia APR fetcher
sqsutil/datafetchers/numia_apr_fetcher.go
Replaces single-attempt fetch with looped retries (count and delay constants), adds per-attempt logging, sleeps between attempts, returns early on success, increments error counter only after all retries fail, adds fmt/time imports.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant F as NumiaAPRFetcher
  participant N as Numia API

  C->>F: GetPoolAPRs()
  rect rgba(200,230,255,0.3)
    note over F: Retry loop (i = 1..N)
    F->>N: Fetch APRs
    alt Success
      N-->>F: APR data
      F-->>C: Map of APRs (return)
    else Failure
      N-->>F: Error
      F->>F: Log attempt error
      opt More retries remaining
        F->>F: Sleep (retry delay)
      end
    end
  end
  alt All retries failed
    F->>F: Increment error counter
    F-->>C: Error (retries exhausted)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through loops, a patient hare,
Retrying calls with careful flair.
If APRs don’t quickly show,
I pause, then try another go.
At last—data!—carrot-bright,
Or count an error in the night.
Thump-thump: resilience done right.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately summarizes the main change—adding linear retry logic to Numia APR queries—without extraneous details, and it is concise and clear.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch roman/linear-retry-numia-apr

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7b5a38 and 237b56d.

📒 Files selected for processing (1)
  • sqsutil/datafetchers/numia_apr_fetcher.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sqsutil/datafetchers/numia_apr_fetcher.go (3)
domain/passthrough/numia_http_client.go (1)
  • NumiaHTTPClient (10-13)
log/logger.go (1)
  • Logger (10-16)
domain/telemetry.go (1)
  • SQSPassthroughNumiaAPRsFetchErrorCounter (235-240)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build
  • GitHub Check: test_coingecko_prices
  • GitHub Check: Run linter
  • GitHub Check: Summary
🔇 Additional comments (3)
sqsutil/datafetchers/numia_apr_fetcher.go (3)

4-4: LGTM! Well-defined retry configuration.

The new imports and retry constants are appropriate. The fixed retry count of 3 with a 1-second delay provides a simple, predictable retry mechanism for transient failures.

Also applies to: 6-6, 15-20


23-24: LGTM! Documentation accurately describes the retry behavior.

The function documentation clearly explains the retry mechanism and error handling.


49-52: LGTM! Proper error handling after exhausting retries.

The error counter is correctly incremented only after all retry attempts fail, and the error message provides helpful context with the retry count.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@p0mvn p0mvn requested a review from pablin-10 October 15, 2025 12:12
logger.Error("Failed to fetch pool APRs,", zap.Error(err), zap.Int("retry", i))

time.Sleep(numiaAPRsFetchRetryDelay)
continue
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unnecessary Delay in Final API Call Retry

The GetFetchPoolAPRsFromNumiaCb function's retry logic includes an unconditional time.Sleep after a failed API call. This sleep occurs even on the final retry attempt, introducing an unnecessary delay before the function returns an error.

Fix in Cursor Fix in Web

Copy link
Collaborator

@pablin-10 pablin-10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants