fix(ratelimiter): return error when olric setup fails

[mesutoezdil]

TL;DR

NewRateLimiter logged the olric.New error and kept going with a nil db, so the next db.Start call panicked and crashed the process. Now it returns a wrapped error, and a stopCaches helper stops the two ttlcache goroutines on all three constructor error paths. go build and go test pass; no new test because olric.New cannot be forced to fail from outside the module.

Issues

Closes #291

Summary by CodeRabbit

• Bug Fixes
  • Improved rate limiter startup reliability by ensuring background caches are stopped when initialization fails.
  • Initialization now fails fast and returns a clear error instead of continuing in a partially configured state.
• Documentation
  • Clarified comment text around rate-limit tier applicability and precedence.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The rate limiter now stops its caches when Olric or store initialization fails, returns a wrapped Olric error instead of continuing with a nil database, and updates comments describing tier behavior.

Changes

Rate limiter initialization

Layer / File(s)	Summary
Handle initialization failures and clarify tier comments src/invocation-plane-services/ratelimiter/rate_limiter.go	NewRateLimiter reuses cache cleanup across Olric and store failure paths, returns a wrapped Olric error, and updates comments for per-user, user-tier, and NCA-tier behavior.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: returning an error when Olric setup fails.
Linked Issues check	✅ Passed	The fix returns Olric init errors instead of continuing with a nil db, addressing the reported startup panic.
Out of Scope Changes check	✅ Passed	The extra cache shutdown and comment tweaks are incidental to the constructor error-handling fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/invocation-plane-services/ratelimiter/rate_limiter.go`:
- Line 304: Add a regression test for the Olric construction failure path in the
relevant rate-limiter startup function, verifying that an `olric.New` error
returns a nil limiter and propagates the same error. Use the repository’s native
test runner; if testing is infeasible, document the reason in the pull request
description.
- Around line 303-304: Remove the zap.L().Error call from the Olric
initialization failure path and continue returning err to the caller. Keep the
existing error propagation behavior in the surrounding initialization function
without adding duplicate logging.
- Around line 301-304: Update the Olric initialization error path after
olric.New in the rate-limiter setup to stop both ttlcache instances with
cache.Stop() and indexedPoliciesCache.Stop() before returning the error.
Preserve the existing error log and return values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 865fa2c6-1ede-4bb7-9cf2-6ea71dda8474

📥 Commits

Reviewing files that changed from the base of the PR and between 5ef95ed and 506a8ea.

📒 Files selected for processing (1)

• src/invocation-plane-services/ratelimiter/rate_limiter.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/invocation-plane-services/ratelimiter/rate_limiter.go`:
- Around line 331-334: In the store initialization error path around NewStore,
call db.Shutdown(context.Background()) on a best-effort basis before returning
the store error, after stopping caches. Preserve the existing error logging and
ensure shutdown failures do not replace or suppress the original NewStore error.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 35fb9aae-c1e3-41bb-9839-594dee98dd38

📥 Commits

Reviewing files that changed from the base of the PR and between 8099447 and 4c15bfa.

📒 Files selected for processing (1)

• src/invocation-plane-services/ratelimiter/rate_limiter.go