From 4a36b4fed43751bfd6cf9cdbc0588b6422c4a1cb Mon Sep 17 00:00:00 2001 From: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Date: Wed, 13 May 2026 11:20:06 +0530 Subject: [PATCH] fix(observability): skip Sentry for param-validation errors (OPENHUMAN-TAURI-20) (#1575) Co-authored-by: Steven Enamakel --- src/core/jsonrpc.rs | 74 +++++++++++++++++++++++++++++++++++---- src/core/jsonrpc_tests.rs | 40 +++++++++++++++++++-- 2 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 40ff4cf8b..8278eb839 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -73,25 +73,51 @@ pub async fn rpc_handler(State(state): State, Json(req): Json err ({}ms): {}", method, ms, display_message); + } else { crate::core::observability::report_error_or_expected( display_message.as_str(), "rpc", "invoke_method", &[("method", method.as_str()), ("elapsed_ms", &ms.to_string())], ); - } else { - tracing::info!("[rpc] {} -> err ({}ms): {}", method, ms, display_message); } ( StatusCode::OK, @@ -176,6 +202,40 @@ fn is_session_expired_error(msg: &str) -> bool { || msg.contains("SESSION_EXPIRED") } +/// Returns `true` when the error message comes from JSON-RPC params validation +/// rather than the underlying handler. +/// +/// Three shapes, all emitted before the handler ever runs: +/// * `"unknown param '' for ."` — `all::validate_params` (extra field) +/// * `"missing required param '': "` — `all::validate_params` (omitted required field) +/// * `"invalid params: expected object or null, got "` — `params_to_object` (wrong params shape) +/// +/// These only fire when caller and server schemas drift at the transport layer +/// — either a frontend on a different release than the running core, or a buggy +/// external client. Reporting them to Sentry produces unactionable noise (we +/// cannot patch an already-shipped install, and the message itself already +/// names the bad field). +/// +/// Note: domain-level validation errors (e.g. type/format checks emitted *inside* +/// a controller's `rpc.rs` handler such as `"param 'x' must be a UUID"`) are +/// intentionally *not* matched here — only the three shapes emitted by the +/// transport-layer validators before the handler runs. Longer-term a typed +/// `RpcError::ParamValidation` variant would remove the string-matching +/// brittleness; the unit tests in `jsonrpc_tests.rs` lock the exact prefixes +/// against the emit sites in `all::validate_params` and `params_to_object`. +/// +/// `starts_with` (not `.contains()`) is deliberate: validator errors are always +/// emitted as the full message body, so an anchored match avoids false positives +/// from upstream handler text that happens to mention `"unknown param"`. The +/// session-expired predicate uses `.contains()` because session-expired markers +/// can appear mid-message — flip these to match and the test +/// `is_param_validation_error_does_not_match_unrelated_errors` will break. +fn is_param_validation_error(msg: &str) -> bool { + msg.starts_with("unknown param '") + || msg.starts_with("missing required param '") + || msg.starts_with("invalid params: ") +} + /// Internal method invocation logic. /// /// It first attempts to match the method name against the static controller diff --git a/src/core/jsonrpc_tests.rs b/src/core/jsonrpc_tests.rs index 5d1d5dded..03667c324 100644 --- a/src/core/jsonrpc_tests.rs +++ b/src/core/jsonrpc_tests.rs @@ -6,8 +6,8 @@ use std::time::Duration; use tokio_util::sync::CancellationToken; use super::{ - build_http_schema_dump, default_state, escape_html, invoke_method, is_session_expired_error, - params_to_object, parse_json_params, rpc_handler, type_name, + build_http_schema_dump, default_state, escape_html, invoke_method, is_param_validation_error, + is_session_expired_error, params_to_object, parse_json_params, rpc_handler, type_name, }; struct EnvVarGuard { @@ -612,6 +612,42 @@ fn is_session_expired_error_does_not_match_unrelated_errors() { assert!(!is_session_expired_error("")); } +#[test] +fn is_param_validation_error_matches_the_three_validator_shapes() { + // Regression guard for OPENHUMAN-TAURI-20: pre-#1467 cores rejected + // `api_key` because it wasn't in the schema yet. The error string + // must keep matching here so it gets logged at info level and never + // reaches Sentry as an unactionable client/server skew event. + assert!(is_param_validation_error( + "unknown param 'api_key' for config.update_model_settings" + )); + // `all::validate_params` — missing required field. + assert!(is_param_validation_error( + "missing required param 'session_id': active session identifier" + )); + // `params_to_object` — params field is the wrong JSON shape. + assert!(is_param_validation_error( + "invalid params: expected object or null, got array" + )); +} + +#[test] +fn is_param_validation_error_does_not_match_unrelated_errors() { + // Handler-side / network / auth failures must still be reported. + assert!(!is_param_validation_error( + "backend returned 401 Unauthorized" + )); + assert!(!is_param_validation_error("network timeout")); + assert!(!is_param_validation_error( + "config.update_model_settings: store write failed" + )); + // Empty and substring-only matches don't qualify either. + assert!(!is_param_validation_error("")); + assert!(!is_param_validation_error( + "rpc failed: unknown param 'x' for ns.fn" + )); +} + #[test] fn is_session_expired_error_matches_missing_backend_session_token() { // Composio / web search / billing / team / webhooks / referral all surface