diff --git a/src/core/dispatch.rs b/src/core/dispatch.rs index 9e6ad82a4..8392d9408 100644 --- a/src/core/dispatch.rs +++ b/src/core/dispatch.rs @@ -82,8 +82,63 @@ pub async fn dispatch( return result; } - log::warn!("[rpc:dispatch] unknown_method method={}", method); - Err(format!("unknown method: {method}")) + // Tier 4: unrecognised method. The JSON-RPC response is unchanged — the + // caller still receives a method-not-found error. Only the *severity* of + // how the transport layer records it differs by class (see + // `jsonrpc::rpc_handler`): known external probes (`is_known_probe_method`) + // are debug-only and never reach Sentry (#3567), while any other unknown + // method is downgraded to a warn-level capture (recorded for triage, no + // page) instead of an error event. Log here at debug with the method name + // so the path stays diagnosable without re-creating the Sentry noise. + if is_known_probe_method(method) { + log::debug!( + "[rpc] unknown_method method={} class=known_probe (debug-only; not reported to Sentry)", + method + ); + } else { + log::debug!( + "[rpc] unknown_method method={} class=unrecognized (reported to Sentry at warn for triage)", + method + ); + } + Err(format!("{UNKNOWN_METHOD_PREFIX}{method}")) +} + +/// Prefix of the error string returned for an unrecognised RPC method. Kept as +/// a shared constant so the emit site (above) and the transport-layer +/// classifier ([`unknown_method_name`]) cannot drift apart. +pub const UNKNOWN_METHOD_PREFIX: &str = "unknown method: "; + +/// Generic external probe / legacy method names that are never real RPC +/// methods and never will be (issue #3567). Infra health-checks and JSON-RPC +/// introspection clients poll these — `rpc.discover` (JSON-RPC service +/// discovery), `list_methods`, liveness `status`, `auth.status`, `config/get` +/// — and each miss previously produced a recurring Sentry ERROR event with +/// zero user impact. The transport layer keeps these debug-only (never +/// captured). The matching health-method *aliases* land separately in +/// `legacy_aliases` (#3566), which depends on this severity change. +const KNOWN_PROBE_METHODS: &[&str] = &[ + "rpc.discover", + "list_methods", + "status", + "auth.status", + "config/get", +]; + +/// Returns `true` when `method` is a known external probe / legacy health name +/// from [`KNOWN_PROBE_METHODS`]. Matched against the *resolved* method name +/// (after legacy-alias rewrite), i.e. the name embedded in the +/// [`UNKNOWN_METHOD_PREFIX`] error string. +pub fn is_known_probe_method(method: &str) -> bool { + KNOWN_PROBE_METHODS.contains(&method) +} + +/// Extracts the offending method name from an unknown-method error string, or +/// `None` if `message` is not an unknown-method error. The transport layer uses +/// this to classify the failure for Sentry severity without re-deriving the +/// method from the request (which may differ post legacy-alias rewrite). +pub fn unknown_method_name(message: &str) -> Option<&str> { + message.strip_prefix(UNKNOWN_METHOD_PREFIX) } /// Handles internal core-level RPC methods. @@ -314,6 +369,51 @@ mod tests { assert_eq!(out, json!({ "ok": true })); } + #[test] + fn is_known_probe_method_matches_allow_list_exactly() { + // Every allow-listed probe / legacy health name is recognised. + for m in [ + "rpc.discover", + "list_methods", + "status", + "auth.status", + "config/get", + ] { + assert!(is_known_probe_method(m), "{m} should be a known probe"); + } + // Genuinely-unknown methods and near-misses are NOT allow-listed, so + // they stay on the warn-for-triage path rather than being silenced. + assert!(!is_known_probe_method("does.not.exist")); + assert!(!is_known_probe_method("core.not_a_real_method")); + assert!(!is_known_probe_method("Status")); // case-sensitive + assert!(!is_known_probe_method("rpc.discover.extra")); // exact match only + assert!(!is_known_probe_method("")); + } + + #[test] + fn unknown_method_name_extracts_from_error_string_only() { + // The classifier round-trips the exact string `dispatch` emits. + let err = format!("{UNKNOWN_METHOD_PREFIX}rpc.discover"); + assert_eq!(unknown_method_name(&err), Some("rpc.discover")); + // Unrelated error strings are not misclassified as unknown-method. + assert_eq!(unknown_method_name("unknown param 'x' for ns.fn"), None); + assert_eq!(unknown_method_name("Session expired"), None); + } + + #[tokio::test] + async fn dispatch_probe_method_still_returns_unknown_method_error() { + // Allow-listed probe names must not be silently "handled" — the caller + // still gets a method-not-found error. Only the Sentry severity (in the + // transport layer) changes; the dispatch contract is unchanged. + let err = dispatch(test_state(), "rpc.discover", json!({})) + .await + .expect_err("probe methods are still unknown to the dispatcher"); + assert_eq!(unknown_method_name(&err), Some("rpc.discover")); + assert!(is_known_probe_method( + unknown_method_name(&err).expect("unknown-method error") + )); + } + #[tokio::test] async fn dispatch_legacy_alias_routes_to_registry() { // openhuman.get_analytics_settings should rewrite to openhuman.config_get_analytics_settings. diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 2804032e9..49526d501 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -132,6 +132,31 @@ pub async fn rpc_handler(State(state): State, Json(req): Json sentry::integrations::tracing::EventFilter::Event, + Level::WARN | Level::INFO => sentry::integrations::tracing::EventFilter::Breadcrumb, + _ => sentry::integrations::tracing::EventFilter::Ignore, + } + }), + ); + let _subscriber_guard = tracing::subscriber::set_default(subscriber); + + // (1) Allow-listed probe → debug-only, never reaches Sentry. + let probe_request = crate::core::types::RpcRequest { + jsonrpc: "2.0".to_string(), + id: json!(1), + method: "rpc.discover".to_string(), + params: json!({}), + }; + let response = rpc_handler(State(default_state()), Json(probe_request)).await; + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("response body"); + let body: serde_json::Value = serde_json::from_slice(&body).expect("json response"); + // (3) Response is the unchanged JSON-RPC method-not-found envelope. + assert_eq!(body["error"]["code"], json!(-32000)); + assert_eq!( + body["error"]["message"], + json!("unknown method: rpc.discover") + ); + assert_eq!(body["error"]["data"], serde_json::Value::Null); + assert!( + transport.fetch_and_clear_events().is_empty(), + "allow-listed probe methods must not reach Sentry" + ); + + // (2) Genuinely-unknown method → still captured, but at warn for triage. + let unknown_request = crate::core::types::RpcRequest { + jsonrpc: "2.0".to_string(), + id: json!(2), + method: "totally.made.up.method".to_string(), + params: json!({}), + }; + let response = rpc_handler(State(default_state()), Json(unknown_request)).await; + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("response body"); + let body: serde_json::Value = serde_json::from_slice(&body).expect("json response"); + // (3) Same unchanged method-not-found envelope for the unknown method. + assert_eq!(body["error"]["code"], json!(-32000)); + assert_eq!( + body["error"]["message"], + json!("unknown method: totally.made.up.method") + ); + assert_eq!(body["error"]["data"], serde_json::Value::Null); + + let events = transport.fetch_and_clear_events(); + assert_eq!( + events.len(), + 1, + "genuinely-unknown methods should still be captured for triage" + ); + assert_eq!(events[0].level, sentry::Level::Warning); + assert_eq!( + events[0].tags.get("domain").map(String::as_str), + Some("rpc") + ); + assert_eq!( + events[0].tags.get("method").map(String::as_str), + Some("totally.made.up.method") + ); } #[test] diff --git a/src/core/observability.rs b/src/core/observability.rs index 1efd4715b..5b24c36c0 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -1831,6 +1831,48 @@ pub(crate) fn report_error_message( ); } +/// Capture a message to Sentry at **warning** severity with structured tags. +/// +/// Mirror of [`report_error_message`] but at `sentry::Level::Warning`: the +/// event is still recorded in Sentry (so it stays available for triage and +/// dashboards) while warning-severity events do not trip the error-rate +/// alert/paging rules that `Level::Error` events do (see the +/// `sentry_tracing_layer` mapping in `core::logging`, where `ERROR` becomes a +/// captured `Event` and `WARN`/`INFO` only a `Breadcrumb`). Use this for +/// transport-boundary conditions worth seeing in aggregate that are never an +/// actionable core defect — e.g. unrecognised RPC method names (#3567). +/// +/// Like [`report_error_message`], capture is an explicit, synchronous +/// `sentry::capture_message` rather than the `sentry-tracing` bridge; the +/// accompanying diagnostic line is tagged with [`REPORT_ERROR_TRACING_TARGET`] +/// so the production layer ignores it and we never double-report. +pub(crate) fn report_warning_message( + message: &str, + domain: &str, + operation: &str, + extra: &[Tag<'_>], +) { + sentry::with_scope( + |scope| { + scope.set_tag("domain", domain); + scope.set_tag("operation", operation); + for (k, v) in extra { + scope.set_tag(k, v); + } + }, + || { + sentry::capture_message(message, sentry::Level::Warning); + tracing::warn!( + target: REPORT_ERROR_TRACING_TARGET, + domain = domain, + operation = operation, + message = %message, + "[observability] {domain}.{operation} warning: {message}" + ); + }, + ); +} + /// Returns true when a Sentry event is a per-attempt provider HTTP failure /// that the reliable-provider layer already handles via retry + fallback. ///