fix(cubestore): do not log rate limit errors as errors in HTTP module

[paveltiunov]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Description of Changes Made (if issue reference is not provided)

"Cache & queue rate limit" errors were flooding the CubeStore error log via the cubestore::http module (~33k Error processing HTTP command entries per 15 minutes on affected deployments). These errors are expected under load and are already returned to the client, so this change downgrades them from error! to warn! at both process_command error-handling sites in http/mod.rs. All other errors are still logged at error level, and the error responses sent to clients are unchanged.

Verified with cargo check -p cubestore.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BwdzVLM36sqhCeT2Mhimro

[claude]

Claude finished @paveltiunov's task in 1m 22s —— View job

---

Code Review

Small, low-risk change that stops rate-limit errors from flooding the error! log while still returning the error to the client. Behavior is preserved; only log severity changes.

Review details — 2 low-severity notes

• Read the diff and surrounding context in rust/cubestore/cubestore/src/http/mod.rs
• Check where "rate limit" error strings originate (open-source BasicProcessRateLimiter is a no-op; the real message comes from a non-OSS implementation, so we can't unit-test the classifier against a real producer here)
• Verify is_wrong_connection() path and client-facing error response are unchanged
• Post review

Observations

1. Code duplication in both branches (low). The if is_rate_limit_error(&e) { debug!(...) } else { error!(...) } blocks repeat the exact same format string and arguments — just the macro differs. log::log! accepts a runtime level and would collapse each pair into a single call, making it obvious the message is identical and the only variable is severity:

let level = if is_rate_limit_error(&e) { log::Level::Debug } else { log::Level::Error };
log::log!(
    level,
    "Error processing HTTP command (connection_id={}): {}\nThe command: {}",
    connection_id.as_deref().unwrap_or("(None)"),
    e.display_with_backtrace(),
    command_text,
);

Not a blocker — the current form is straightforward — just a readability win.

2. Substring classifier is fragile (low, informational). e.message.contains("rate limit") is a plain, case-sensitive substring match. Two things to be aware of:

• It won't match "Rate limit" / "RATE LIMIT" if the message casing ever varies. Consider to_ascii_lowercase().contains("rate limit") or checking a couple of known prefixes explicitly.
• It could in principle match unrelated errors whose message happens to contain those two words (e.g. a user-supplied query fragment echoed back in an error). Low probability, but worth noting — a stronger signal would be an error variant/code on CubeError, since the producer lives in a separate crate. Not necessary to change for this PR, but a follow-up ticket to tag rate-limit errors structurally would remove the brittleness for good.

Non-issues

• Downgrading to debug! is the right call — the client already receives the error response, so operators don't lose visibility into what happened for a given request; only the log noise goes away.
• The is_wrong_connection() branch and the HttpCommand::Error / HttpCommand::CloseConnection mapping are untouched. Good.
• No test added, which the PR checklist acknowledges. Given the rate-limit producer is out of tree, a meaningful test would need to construct a CubeError with a synthetic message — feasible but low-value; fine to skip.

· [Branch](https://github.com/cube-js/cube/tree/claude/rate-limit-errors-http-s1m91v)

[claude]

Claude finished @paveltiunov's task in 1m 57s —— View job

---

Code review

Verdict: looks good in intent — the two log sites do get downgraded and the client response is unchanged. Two concerns worth addressing before merge, plus one nit.

Details (1 medium, 1 low, 1 nit)

1. Fragile substring match (medium) — http/mod.rs:78

e.message.contains("rate limit") is case-sensitive, and rg "rate limit" rust/ returns only this file — the phrase originates outside this crate, so the classifier is an implicit cross-repo contract with no anchor. Any capitalization ("Rate limit exceeded") or wording tweak upstream silently re-enables the error-log flood.

Prefer either a case-insensitive check or, better, a CubeErrorCauseType::RateLimit variant tagged at the producing site and exposed via e.is_rate_limit(), mirroring the existing is_wrong_connection() pattern at lib.rs:145. Inline comment posted.

2. Duplicated log! blocks (low) — http/mod.rs:340-357, 444-456

Both sites duplicate the whole format-argument block just to switch macros. log::log!(level, ...) accepts the level as a value, letting each site collapse to one call and keeping the format string canonical so the two branches can't drift. Inline comment with a suggested edit posted at the first site.

3. PR description is stale (nit)

Description says the change downgrades to debug!, but the follow-up commit lands on warn!. Worth updating the body so the merge commit / release notes read correctly.

• [branch](https://github.com/cube-js/cube/tree/claude/rate-limit-errors-http-s1m91v)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.44%. Comparing base (3013a46) to head (32b7a8f).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #11293   +/-   ##
=======================================
  Coverage   59.44%   59.44%           
=======================================
  Files         216      216           
  Lines       17474    17474           
  Branches     3539     3539           
=======================================
  Hits        10387    10387           
  Misses       6581     6581           
  Partials      506      506           

Flag	Coverage Δ
cube-backend	59.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude finished @paveltiunov's task in 44s —— View job

---

Code review

Verdict: LGTM. Both prior review threads (case-sensitivity, log-call duplication) were addressed in 32b7a8f, and no new issues surfaced. Resolved the two stale threads.

Details

• Re-diff against origin/master for rust/cubestore/cubestore/src/http/mod.rs
• Verify case-insensitive matcher: e.message.to_ascii_lowercase().contains("rate limit") — matches "Rate limit", "RATE LIMIT", etc.
• Verify log::log!(level, ...) collapse at both sites (L337-347 and L438-448); format strings are now single-sourced per site
• Confirm client-facing response paths (HttpCommand::Error / CloseConnection) are unchanged
• Resolve the two outdated claude threads

Notes

• The is_rate_limit_error helper is small, well-commented, and localized to this module — appropriate scope for a log-noise fix.
• The substring-based classifier is still an implicit cross-repo contract (the producer of "rate limit" lives outside this crate), but with case-insensitivity that fragility is greatly reduced. A structural CubeErrorCauseType::RateLimit variant remains the cleaner long-term option — worth a follow-up ticket, not a blocker for this PR.
• PR body still says "downgrades them from error! to warn!" — that now matches the final state (earlier debug! was superseded). No stale text left.
• Codecov shows no coverage delta; no test added, which is reasonable given the producer is out of tree.

· [Branch](https://github.com/cube-js/cube/tree/claude/rate-limit-errors-http-s1m91v)