From 386b0025edc1aedf31b2e67352313ec6435fb527 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Wed, 13 May 2026 08:26:24 +0530 Subject: [PATCH] fix(observability/auth): handle JWT-required expiry path and normalize backend API base URLs (#1551) Co-authored-by: Steven Enamakel --- src/api/rest.rs | 65 +++++++++++++++++++++++++++++---------- src/api/rest_tests.rs | 33 ++++++++++++++++++++ src/core/jsonrpc.rs | 6 ++++ src/core/jsonrpc_tests.rs | 18 +++++++++++ 4 files changed, 105 insertions(+), 17 deletions(-) diff --git a/src/api/rest.rs b/src/api/rest.rs index dc30af87f..168419745 100644 --- a/src/api/rest.rs +++ b/src/api/rest.rs @@ -203,8 +203,22 @@ pub struct BackendOAuthClient { impl BackendOAuthClient { /// Creates a new `BackendOAuthClient` with the given API base URL. + /// + /// Any path, query, or fragment in `api_base` is stripped so that + /// `Url::join` always resolves root-relative REST paths correctly. + /// This guards against callers who pass a full LLM completions URL + /// (e.g. `https://host/v1/chat/completions`) instead of just the origin: + /// without stripping, `join("teams/me/usage")` would produce the wrong + /// path `/v1/chat/teams/me/usage` via RFC 3986 relative resolution. pub fn new(api_base: &str) -> Result { - let base = Url::parse(api_base.trim()).context("Invalid API base URL")?; + let mut base = Url::parse(api_base.trim()).context("Invalid API base URL")?; + anyhow::ensure!( + matches!(base.scheme(), "http" | "https") && base.host_str().is_some(), + "API base URL must be an absolute http(s) URL with host" + ); + base.set_path(""); + base.set_query(None); + base.set_fragment(None); let client = build_backend_reqwest_client()?; Ok(Self { client, base }) } @@ -429,23 +443,40 @@ impl BackendOAuthClient { let status = response.status(); let text = response.text().await.unwrap_or_default(); if !status.is_success() { - let status_str = status.as_u16().to_string(); - crate::core::observability::report_error( - format!( - "{} {} failed ({status}): {text}", + let status_code = status.as_u16(); + let status_str = status_code.to_string(); + // 502/503/504 are transient infrastructure errors (proxy/CDN/backend + // temporarily unavailable). They are not code bugs and callers already + // implement retry/disable logic, so skip Sentry to avoid noise. + let is_transient_infra = matches!(status_code, 502 | 503 | 504); + if is_transient_infra { + tracing::warn!( + method = method.as_str(), + path = url.path(), + status = status_code, + "[backend_api] transient {status} on {} {} — not reporting to Sentry", method.as_str(), - url.path() - ) - .as_str(), - "backend_api", - "authed_json", - &[ - ("method", method.as_str()), - ("path", url.path()), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); + url.path(), + ); + } else { + crate::core::observability::report_error( + format!( + "{} {} failed ({status}); response_body_len={}", + method.as_str(), + url.path(), + text.len() + ) + .as_str(), + "backend_api", + "authed_json", + &[ + ("method", method.as_str()), + ("path", url.path()), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } anyhow::bail!( "{} {} failed ({status}): {text}", method.as_str(), diff --git a/src/api/rest_tests.rs b/src/api/rest_tests.rs index 1b425a940..a51f63e5a 100644 --- a/src/api/rest_tests.rs +++ b/src/api/rest_tests.rs @@ -215,6 +215,39 @@ async fn backend_client_sends_x_core_version_on_auth_requests() { ); } +// Regression: OPENHUMAN-TAURI-8K / Sentry issue 7473650958. +// When config.api_url is a full LLM completions URL (e.g. /v1/chat/completions), +// Url::join used to produce wrong paths like /v1/chat/teams/me/usage instead of +// /teams/me/usage — BackendOAuthClient::new must strip the path to prevent this. +#[test] +fn new_strips_path_from_completions_url() { + let client = BackendOAuthClient::new("https://api.tinyhumans.ai/v1/chat/completions").unwrap(); + let url = client.url_for("/teams/me/usage").unwrap(); + assert_eq!(url.path(), "/teams/me/usage"); +} + +#[test] +fn new_strips_path_from_openai_style_url() { + let client = BackendOAuthClient::new("https://api.openai.com/v1/chat/completions").unwrap(); + let url = client.url_for("/teams/me/usage").unwrap(); + assert_eq!(url.path(), "/teams/me/usage"); + assert_eq!(url.host_str(), Some("api.openai.com")); +} + +#[test] +fn new_works_with_bare_origin() { + let client = BackendOAuthClient::new("https://api.tinyhumans.ai").unwrap(); + let url = client.url_for("/teams/me/usage").unwrap(); + assert_eq!(url.path(), "/teams/me/usage"); +} + +#[test] +fn new_works_with_trailing_slash() { + let client = BackendOAuthClient::new("https://api.tinyhumans.ai/").unwrap(); + let url = client.url_for("/teams/me/usage").unwrap(); + assert_eq!(url.path(), "/teams/me/usage"); +} + #[tokio::test] async fn backend_raw_client_inherits_x_core_version_default_header() { let (base_url, captured) = spawn_header_capture_server().await; diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 82559c7a3..4813ab7d0 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -138,11 +138,17 @@ pub async fn invoke_method(state: AppState, method: &str, params: Value) -> Resu /// repeatedly reporting this as a hard error to Sentry. See #1465-ish: users /// stuck on the onboarding `SkillsStep` would spam `composio_list_connections` /// failures every 5 s without ever being bounced back to the login screen. +/// +/// "session JWT required" covers the case where a prior 401 already cleared the +/// token and the very next RPC call (e.g. `channels_telegram_login_start`) finds +/// no JWT in the store. This is the same auth-boundary condition, just surfaced +/// as a local guard rather than a backend response. fn is_session_expired_error(msg: &str) -> bool { let lower = msg.to_lowercase(); (lower.contains("401") && lower.contains("unauthorized")) || lower.contains("invalid token") || lower.contains("no backend session token") + || lower.contains("session jwt required") || msg.contains("SESSION_EXPIRED") } diff --git a/src/core/jsonrpc_tests.rs b/src/core/jsonrpc_tests.rs index 3aa845ff0..f482d1ae6 100644 --- a/src/core/jsonrpc_tests.rs +++ b/src/core/jsonrpc_tests.rs @@ -597,6 +597,24 @@ fn is_session_expired_error_matches_missing_backend_session_token() { assert!(is_session_expired_error("NO BACKEND SESSION TOKEN")); } +#[test] +fn is_session_expired_error_matches_session_jwt_required() { + // Regression: Sentry issue 7472592145. + // A prior 401 clears the stored JWT; the very next RPC call (e.g. + // channels_telegram_login_start) finds no token and returns "session JWT + // required; complete login first". This is the same auth-boundary condition + // and must not be reported to Sentry. + assert!(is_session_expired_error( + "session JWT required; complete login first" + )); + assert!(is_session_expired_error( + "session JWT required; complete login and store_session first" + )); + assert!(is_session_expired_error("session JWT required")); + // Case-insensitive. + assert!(is_session_expired_error("SESSION JWT REQUIRED")); +} + #[test] fn escape_html_escapes_all_special_chars() { let raw = r#""#;