fix(rpc): normalize legacy un-namespaced method names in dispatcher (OPENHUMAN-TAURI-9Q)#1541
Open
oxoxDev wants to merge 1 commit into
Open
Conversation
Production cores log 'unknown method' errors for un-namespaced legacy method names that bypass frontend normalization. This adds a normalization layer in `src/core/dispatch.rs` that maps legacy names to their canonical equivalents, mirroring the `LEGACY_METHOD_ALIASES` table in the frontend. Specifically: - `openhuman.update_analytics_settings` -> `openhuman.config_update_analytics_settings` - And 15 other mappings. A debug log is emitted when normalization fires for monitoring. Added unit tests covering all aliases and integration with `dispatch`. Co-authored-by: oxoxDev <164490987+oxoxDev@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds transparent legacy RPC method-name rewriting to the dispatch function. A new private helper maps old ChangesLegacy RPC method name normalization in dispatch
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core/dispatch.rs (1)
34-38: ⚡ Quick winUse dispatch-scoped structured fields in the normalization debug log
Please align this log line with the existing dispatcher log format for grepability/correlation (
[rpc:dispatch]+ key/value fields).Proposed diff
- log::debug!( - "[rpc] legacy method '{}' rewritten to '{}'", - method, - canonical - ); + log::debug!( + "[rpc:dispatch] normalized_legacy_method legacy_method={} canonical_method={}", + method, + canonical + );As per coding guidelines, "Use log / tracing at debug / trace level; include stable grep-friendly prefixes (
[domain],[rpc]) and correlation fields."🤖 Prompt for 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. In `@src/core/dispatch.rs` around lines 34 - 38, The debug log call using log::debug! that currently prints "[rpc] legacy method '{}' rewritten to '{}'" should be changed to the dispatcher-scoped structured format used elsewhere: replace the free-form message in the log::debug! invocation (the one referencing the variables method and canonical) with a grep-friendly prefix "[rpc:dispatch]" and include stable key/value fields (e.g., method= and canonical=) so the output becomes "[rpc:dispatch]" plus explicit method=... canonical=... fields for correlation and consistent grepability; update the log::debug! call that references method and canonical accordingly.
🤖 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.
Nitpick comments:
In `@src/core/dispatch.rs`:
- Around line 34-38: The debug log call using log::debug! that currently prints
"[rpc] legacy method '{}' rewritten to '{}'" should be changed to the
dispatcher-scoped structured format used elsewhere: replace the free-form
message in the log::debug! invocation (the one referencing the variables method
and canonical) with a grep-friendly prefix "[rpc:dispatch]" and include stable
key/value fields (e.g., method= and canonical=) so the output becomes
"[rpc:dispatch]" plus explicit method=... canonical=... fields for correlation
and consistent grepability; update the log::debug! call that references method
and canonical accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 825bda9b-4951-43d6-98fa-73a2f06c7d66
📒 Files selected for processing (1)
src/core/dispatch.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LEGACY_METHOD_ALIASEStable atapp/src/services/rpcMethods.ts:23-43as defense-in-depth.normalizeRpcMethod) now route to the canonical handler instead of erroringunknown method.Problem
Production cores log
rpc.invoke_method failed: unknown method: openhuman.update_analytics_settings— OPENHUMAN-TAURI-9Q (~19 events). The frontend shipsLEGACY_METHOD_ALIASESwithnormalizeRpcMethod, but at least one call path bypasses normalization and hits the Rust dispatcher raw. Result: legitimate config-domain calls fail withunknown method.Solution
src/core/dispatch.rs— newnormalize_legacy_method(method) -> Option<&'static str>mirroring the TS alias table; pre-pass indispatch()rewrites legacy → canonical and emits atracing::debug!so we can monitor leftover legacy callers.openhuman.update_analytics_settings→openhuman.config_update_analytics_settings,openhuman.ping→core.ping.normalizeRpcMethodleft in place — both layers, defense-in-depth.Submission Checklist
Nonetest, dispatch smoke foropenhuman.pingandopenhuman.get_analytics_settingsdispatch(test_state(), …)N/A: backend defense-in-depth, no new user-visible feature row## Related—N/A: behaviour-only changeN/A: no release-cut surface touchedClosesin the## RelatedsectionImpact
debug!log surfaces any caller still on legacy names so we can chase them down.&str → Option<&'static str>mapping.matchon&str, allocation-free, single lookup per RPC call.Related
app/src/services/rpcMethods.ts:23-43debug!log over a release cycle to identify which call sites still emit legacy names, then fix them in TS so we can deprecate this Rust-side normalizationAI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix-legacy-rpc-normalization-7450370644868266100Validation Run
pnpm --filter openhuman-app format:check— N/A: noapp/changespnpm typecheck— N/A: no TS changescargo test --lib core::dispatch::tests::test_normalize_legacy_method_all_aliasescargo test --lib core::dispatch::tests::test_normalize_legacy_method_none_for_unknown_or_canonicalcargo test --lib core::dispatch::tests::dispatch_legacy_ping_rewrites_and_succeedscargo test --lib core::dispatch::tests::dispatch_legacy_alias_routes_to_registryapp/src-tauri/changesValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
unknown method.unknown methodSentry errors now resolve transparently.Parity Contract
LEGACY_METHOD_ALIASESexactly (16 pairs, verified by parametric test).Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Tests