diff --git a/src/api/rest.rs b/src/api/rest.rs index 0311dc5597..ba8165a7c3 100644 --- a/src/api/rest.rs +++ b/src/api/rest.rs @@ -40,6 +40,15 @@ pub enum BackendApiError { /// Request path the 401 came back from (no query string). path: String, }, + /// `GET /announcements/latest` returned 404. The announcements feature is + /// a best-effort, cosmetic fetch (`app/src/services/announcementService.ts`: + /// "a missing announcement is never worth surfacing an error for") — a 404 + /// here means "no announcement", not a code bug. Callers should degrade to + /// `null` and skip the retry-as-error path. Targets `TAURI-RUST-HW0` + /// (`backend_api`/`authed_json`) and `TAURI-RUST-KHX` (`rpc`/`invoke_method` + /// re-wrap) — one failure reported at two layers, ~452 events / 19 users. + #[error("no announcement available (404 on /announcements/latest)")] + AnnouncementNotFound, } /// Flatten an `authed_json` error onto the JSON-RPC `String` channel. @@ -94,6 +103,16 @@ fn parse_message_path(path: &str) -> Option<(&str, &str)> { None } +/// `true` when `path` is `/announcements/latest`, tolerant of an arbitrary +/// base-path prefix (e.g. `/api/v1/announcements/latest`) — same +/// prefix-tolerant reasoning as [`parse_message_path`] (OPENHUMAN-TAURI-R7): +/// a `BACKEND_URL` override with a path prefix must not cause this route's +/// 404 to silently fall through to the generic `report_error` path. +fn is_announcements_latest_path(path: &str) -> bool { + let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + matches!(segments.as_slice(), [.., "announcements", "latest"]) +} + const CLIENT_VERSION_HEADER_MAX_LEN: usize = 64; /// Max bytes of the `body_shape` key-name list echoed into the `authed_json` @@ -702,6 +721,23 @@ impl BackendOAuthClient { url.path(), ); } + + // 404 on `/announcements/latest` means "no announcement" for + // this best-effort, cosmetic feature — not a code bug. Surface + // a typed `BackendApiError::AnnouncementNotFound` so the caller + // (`announcements::ops::get_latest_announcement`) can degrade to + // `null` instead of propagating an error, without funneling the + // 404 into `report_error`. Targets `TAURI-RUST-HW0` / `TAURI-RUST-KHX`. + if method == Method::GET && is_announcements_latest_path(url.path()) { + tracing::info!( + domain = "backend_api", + operation = "authed_json", + "[backend_api] announcement-not-found 404 on {} {} — surfacing typed error", + method.as_str(), + url.path(), + ); + return Err(anyhow::Error::new(BackendApiError::AnnouncementNotFound)); + } } // These are transient infrastructure errors (proxy/CDN/backend diff --git a/src/api/rest_tests.rs b/src/api/rest_tests.rs index 376e4f3bbf..3a968afce9 100644 --- a/src/api/rest_tests.rs +++ b/src/api/rest_tests.rs @@ -1,6 +1,7 @@ use super::{ - backend_api_body_shape, flatten_authed_error, key_bytes_from_string, parse_message_path, - sanitize_client_version, BackendApiError, BackendOAuthClient, BACKEND_API_BODY_SHAPE_MAX_BYTES, + backend_api_body_shape, flatten_authed_error, is_announcements_latest_path, + key_bytes_from_string, parse_message_path, sanitize_client_version, BackendApiError, + BackendOAuthClient, BACKEND_API_BODY_SHAPE_MAX_BYTES, }; use axum::extract::State; use axum::http::HeaderMap; @@ -358,6 +359,87 @@ async fn authed_json_surfaces_message_not_found_on_404() { assert_eq!(message_id, "abc"); } +#[tokio::test] +async fn authed_json_surfaces_announcement_not_found_on_404() { + // TAURI-RUST-HW0 / TAURI-RUST-KHX: 404 on `/announcements/latest` must + // surface a typed `BackendApiError::AnnouncementNotFound` (so the caller + // can degrade to `null`) instead of a generic non-2xx error. + let app = Router::new().route( + "/announcements/latest", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "Not Found") }), + ); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let base_url = format!("http://{addr}"); + let client = BackendOAuthClient::new(&base_url).unwrap(); + + let err = client + .authed_json("mock-jwt", Method::GET, "/announcements/latest", None) + .await + .unwrap_err(); + let typed = err.downcast_ref::().unwrap(); + assert!(matches!(typed, BackendApiError::AnnouncementNotFound)); +} + +#[tokio::test] +async fn authed_json_only_classifies_get_announcements_latest_as_not_found() { + // Defense-in-depth: a 404 on a *different* path must not be misclassified + // as AnnouncementNotFound just because it shares a prefix/suffix. + let app = Router::new().route( + "/announcements/latest/extra", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "Not Found") }), + ); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let base_url = format!("http://{addr}"); + let client = BackendOAuthClient::new(&base_url).unwrap(); + + let err = client + .authed_json("mock-jwt", Method::GET, "/announcements/latest/extra", None) + .await + .unwrap_err(); + assert!(err.downcast_ref::().is_none()); +} + +#[tokio::test] +async fn authed_json_surfaces_announcement_not_found_with_base_path_prefix() { + // OPENHUMAN-TAURI-R7-style regression: a BACKEND_URL/path override that + // makes the resolved path `/api/v1/announcements/latest` must still + // classify as AnnouncementNotFound, not fall through to a generic error. + let app = Router::new().route( + "/api/v1/announcements/latest", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "Not Found") }), + ); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let base_url = format!("http://{addr}"); + let client = BackendOAuthClient::new(&base_url).unwrap(); + + let err = client + .authed_json( + "mock-jwt", + Method::GET, + "/api/v1/announcements/latest", + None, + ) + .await + .unwrap_err(); + let typed = err.downcast_ref::().unwrap(); + assert!(matches!(typed, BackendApiError::AnnouncementNotFound)); +} + #[tokio::test] async fn authed_json_surfaces_unauthorized_on_401() { // OPENHUMAN-TAURI-4K8: 401 on any authed backend endpoint must surface a @@ -582,6 +664,23 @@ fn flatten_authed_error_does_not_swallow_message_not_found() { ); } +#[test] +fn flatten_authed_error_does_not_swallow_announcement_not_found() { + // `announcements::ops::get_latest_announcement` intercepts + // `AnnouncementNotFound` before it ever reaches `flatten_authed_error`, but + // this is defense-in-depth: if a future caller skips that interception, + // `flatten_authed_error` must still preserve the typed state's Display + // text rather than collapsing it into the session-expiry sentinel. + let err = anyhow::Error::new(BackendApiError::AnnouncementNotFound); + let flat = flatten_authed_error(err); + + assert!(!flat.contains("SESSION_EXPIRED"), "must not map: {flat}"); + assert!( + flat.contains("no announcement available"), + "display preserved: {flat}" + ); +} + #[tokio::test] async fn authed_json_403_is_not_demoted_to_unauthorized() { // 403 (Forbidden) is a genuine authorization/permission problem — the @@ -701,6 +800,34 @@ fn parse_message_path_non_message_path_returns_none() { assert_eq!(parse_message_path(""), None); } +#[test] +fn is_announcements_latest_path_matches_canonical_form() { + assert!(is_announcements_latest_path("/announcements/latest")); +} + +#[test] +fn is_announcements_latest_path_tolerates_base_path_prefix() { + // Same OPENHUMAN-TAURI-R7 reasoning as parse_message_path: a BACKEND_URL + // override with a path prefix must not defeat the 404 classification. + assert!(is_announcements_latest_path("/api/v1/announcements/latest")); + assert!(is_announcements_latest_path("/v2/api/announcements/latest")); +} + +#[test] +fn is_announcements_latest_path_trailing_slash() { + assert!(is_announcements_latest_path("/announcements/latest/")); +} + +#[test] +fn is_announcements_latest_path_rejects_other_paths() { + assert!(!is_announcements_latest_path("/announcements/latest/extra")); + assert!(!is_announcements_latest_path("/announcements")); + assert!(!is_announcements_latest_path("/latest")); + assert!(!is_announcements_latest_path("/auth/profile")); + assert!(!is_announcements_latest_path("/")); + assert!(!is_announcements_latest_path("")); +} + // ── authed_json defense-in-depth: PATCH 404 with base-path prefix ─────────── #[tokio::test] diff --git a/src/openhuman/announcements/ops.rs b/src/openhuman/announcements/ops.rs index 11775a2dbc..58a1db497c 100644 --- a/src/openhuman/announcements/ops.rs +++ b/src/openhuman/announcements/ops.rs @@ -10,7 +10,7 @@ use reqwest::Method; use serde_json::Value; use crate::api::config::effective_backend_api_url; -use crate::api::BackendOAuthClient; +use crate::api::{BackendApiError, BackendOAuthClient}; use crate::openhuman::config::Config; use crate::rpc::RpcOutcome; @@ -20,20 +20,63 @@ fn require_token(config: &Config) -> Result { crate::openhuman::credentials::session_support::require_live_session_token(config) } -async fn get_authed_value(config: &Config, method: Method, path: &str) -> Result { - let token = require_token(config)?; - let api_url = effective_backend_api_url(&config.api_url); - let client = BackendOAuthClient::new(&api_url).map_err(|e| e.to_string())?; - client - .authed_json(&token, method, path, None) - .await - .map_err(crate::api::flatten_authed_error) +/// `true` when `err` is the typed `BackendApiError::AnnouncementNotFound` 404 +/// (see `src/api/rest.rs`) — the backend has no announcement for this user, +/// which is a normal outcome for this best-effort feature, not a failure. +fn is_announcement_not_found(err: &anyhow::Error) -> bool { + matches!( + err.downcast_ref::(), + Some(BackendApiError::AnnouncementNotFound) + ) } /// Fetch the latest active announcement for the signed-in user. /// Maps to `GET /announcements/latest`. The backend returns the announcement /// object or `null` when nothing qualifies; both pass through verbatim. +/// +/// A 404 (`BackendApiError::AnnouncementNotFound`) is folded into that same +/// "no announcement" contract instead of propagating as an error — this +/// feature is best-effort/cosmetic, and surfacing the 404 as a hard failure +/// flooded Sentry with no actionable signal (TAURI-RUST-HW0, TAURI-RUST-KHX). +/// Any other error (5xx, malformed response, session expiry, …) still +/// propagates via `flatten_authed_error` and still reaches Sentry. pub async fn get_latest_announcement(config: &Config) -> Result, String> { - let data = get_authed_value(config, Method::GET, "/announcements/latest").await?; - Ok(RpcOutcome::single_log(data, "latest announcement fetched")) + let token = require_token(config)?; + let api_url = effective_backend_api_url(&config.api_url); + let client = BackendOAuthClient::new(&api_url).map_err(|e| e.to_string())?; + + match client + .authed_json(&token, Method::GET, "/announcements/latest", None) + .await + { + Ok(data) => Ok(RpcOutcome::single_log(data, "latest announcement fetched")), + Err(err) if is_announcement_not_found(&err) => Ok(RpcOutcome::single_log( + Value::Null, + "no announcement available (404)", + )), + Err(err) => Err(crate::api::flatten_authed_error(err)), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn announcement_not_found_error_is_detected() { + let err = anyhow::Error::new(BackendApiError::AnnouncementNotFound); + assert!(is_announcement_not_found(&err)); + } + + #[test] + fn other_backend_errors_are_not_announcement_not_found() { + let err = anyhow::Error::new(BackendApiError::Unauthorized { + method: "GET".to_string(), + path: "/announcements/latest".to_string(), + }); + assert!(!is_announcement_not_found(&err)); + + let plain = anyhow::anyhow!("GET /announcements/latest failed (500): boom"); + assert!(!is_announcement_not_found(&plain)); + } }