mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(announcements): degrade 404 on /announcements/latest to null instead of erroring (#4417)
This commit is contained in:
@@ -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
|
||||
|
||||
+129
-2
@@ -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::<BackendApiError>().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::<BackendApiError>().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::<BackendApiError>().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]
|
||||
|
||||
@@ -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<String, String> {
|
||||
crate::openhuman::credentials::session_support::require_live_session_token(config)
|
||||
}
|
||||
|
||||
async fn get_authed_value(config: &Config, method: Method, path: &str) -> Result<Value, String> {
|
||||
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::<BackendApiError>(),
|
||||
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<RpcOutcome<Value>, 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));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user