diff --git a/crates/arr-api/src/movies.rs b/crates/arr-api/src/movies.rs index b315b78..b8ce472 100644 --- a/crates/arr-api/src/movies.rs +++ b/crates/arr-api/src/movies.rs @@ -159,7 +159,12 @@ pub enum ApiError { NoTrailer, Conflict(String), Invalid(String), + /// The SQLite handle is gone. Distinct from [`Self::Upstream`] so a + /// TMDB or Prowlarr fault never reads as a database fault (issue #168). Unavailable, + /// An upstream service could not be reached, is misconfigured or is + /// unconfigured. The payload names it. + Upstream(&'static str), Database(String), /// A library delete that could not touch the disk. Named separately from /// [`Self::Database`] because the row is still there and a retry is the @@ -184,6 +189,10 @@ impl IntoResponse for ApiError { StatusCode::SERVICE_UNAVAILABLE, "database unavailable".into(), ), + Self::Upstream(name) => ( + StatusCode::SERVICE_UNAVAILABLE, + format!("{name} unavailable"), + ), Self::Database(error) => { tracing::error!(%error, "API database error"); (StatusCode::INTERNAL_SERVER_ERROR, "database error".into()) @@ -554,7 +563,7 @@ pub async fn search( } state .send_movie_command(MovieCommand::Search { movie_id: id }) - .map_err(|_| ApiError::Unavailable)?; + .map_err(|_| ApiError::Upstream("daemon"))?; Ok((StatusCode::ACCEPTED, Json(Accepted { accepted: true }))) } @@ -671,7 +680,7 @@ pub async fn grab( movie_id, release_id, }) - .map_err(|_| ApiError::Unavailable)?; + .map_err(|_| ApiError::Upstream("daemon"))?; Ok((StatusCode::ACCEPTED, Json(Accepted { accepted: true }))) } diff --git a/crates/arr-api/src/search.rs b/crates/arr-api/src/search.rs index 33c82c8..73dc4ce 100644 --- a/crates/arr-api/src/search.rs +++ b/crates/arr-api/src/search.rs @@ -36,9 +36,23 @@ pub struct SearchResponse { /// In-library hits, grouped first (§9.2): movies and series by title. pub library: Vec, pub tmdb: Vec, + /// Why the `tmdb` group is what it is. An unreachable TMDB must not cost + /// the operator the library half (issue #168), so the group goes empty + /// while this says why: `"ok"` with an empty group means TMDB answered + /// and found nothing — a different world from `"unavailable"`. + pub tmdb_status: TmdbStatus, pub manual: Option, } +#[derive(Debug, Clone, Copy, Serialize, ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum TmdbStatus { + /// The `tmdb` group carries TMDB's answer — possibly an empty one. + Ok, + /// TMDB could not be reached or is unconfigured. + Unavailable, +} + /// One in-library hit. Tagged so a client can branch on what it found /// without re-deriving it from fields. #[derive(Debug, Clone, Serialize, ToSchema)] @@ -167,6 +181,7 @@ pub async fn search( kind, library: Vec::new(), tmdb: Vec::new(), + tmdb_status: TmdbStatus::Ok, manual: Some(input.to_owned()), })); } @@ -211,8 +226,27 @@ pub async fn search( }; library.extend(series_rows.into_iter().map(LibraryResult::Series)); - let tmdb = tmdb_client(&state)?; - let results = search_tmdb(&tmdb, kind, input).await?; + // A degraded TMDB must not discard the library results already in hand + // (issue #168): the group goes empty and `tmdb_status` says why, so the + // operator can tell "TMDB found nothing" from "TMDB is unreachable". + let (results, tmdb_status) = match tmdb_client(&state) { + Ok(tmdb) => match search_tmdb(&tmdb, kind, input).await { + Ok(found) => (found, TmdbStatus::Ok), + // TMDB answered and has no such title — an empty group that is + // not a fault. + Err(ApiError::NotFound) => (Vec::new(), TmdbStatus::Ok), + Err(ApiError::Upstream(name)) => { + tracing::warn!(upstream = name, "unified search: upstream unavailable"); + (Vec::new(), TmdbStatus::Unavailable) + } + Err(error) => return Err(error), + }, + Err(ApiError::Upstream(name)) => { + tracing::warn!(upstream = name, "unified search: upstream unavailable"); + (Vec::new(), TmdbStatus::Unavailable) + } + Err(error) => return Err(error), + }; if matches!(kind, SearchInputKind::ImdbId) { for result in &results { match result { @@ -240,6 +274,7 @@ pub async fn search( kind, library, tmdb: results, + tmdb_status, manual: None, })) } @@ -415,7 +450,7 @@ async fn movie_releases( let indexers = prowlarr .indexers() .await - .map_err(|_| ApiError::Unavailable)?; + .map_err(|_| ApiError::Upstream("prowlarr"))?; let blacklist = Blacklist::load(database.pool()).await?; let policy = loaded.policy; let overrides = loaded.overrides; @@ -512,7 +547,7 @@ async fn episode_releases( let indexers = prowlarr .indexers() .await - .map_err(|_| ApiError::Unavailable)?; + .map_err(|_| ApiError::Upstream("prowlarr"))?; let blacklist = Blacklist::load(database.pool()).await?; let mut classified = Vec::new(); @@ -634,8 +669,9 @@ fn prowlarr_client(state: &AppState) -> Result { let api_key = upstreams .prowlarr_api_key .clone() - .ok_or(ApiError::Unavailable)?; - ProwlarrClient::new(upstreams.prowlarr_url.clone(), api_key).map_err(|_| ApiError::Unavailable) + .ok_or(ApiError::Upstream("prowlarr"))?; + ProwlarrClient::new(upstreams.prowlarr_url.clone(), api_key) + .map_err(|_| ApiError::Upstream("prowlarr")) } pub(crate) fn tmdb_client(state: &AppState) -> Result { @@ -643,17 +679,17 @@ pub(crate) fn tmdb_client(state: &AppState) -> Result ApiError { match error { arr_meta::Error::NotFound { .. } => ApiError::NotFound, - _ => ApiError::Unavailable, + _ => ApiError::Upstream("tmdb"), } } @@ -781,6 +817,23 @@ mod tests { async fn application( tmdb: &MockServer, prowlarr: &MockServer, + ) -> (tempfile::TempDir, AppState, String) { + application_with( + tmdb.uri(), + Some("tmdb-key".into()), + Some("prowlarr-key".into()), + prowlarr.uri(), + ) + .await + } + + /// Like [`application`] but with each upstream knob explicit, so tests + /// can take TMDB or the API keys away. + async fn application_with( + tmdb_url: String, + tmdb_key: Option, + prowlarr_key: Option, + prowlarr_url: String, ) -> (tempfile::TempDir, AppState, String) { let dir = tempfile::tempdir().expect("tempdir"); let database = arr_db::Db::connect(dir.path().join("arr.db")) @@ -788,10 +841,10 @@ mod tests { .expect("database"); database.migrate().await.expect("migrate"); let state = AppState::new( - Upstreams::new(prowlarr.uri(), "http://127.0.0.1:1".into()) - .with_prowlarr_api_key(Some("prowlarr-key".into())) - .with_tmdb_url(tmdb.uri()) - .with_tmdb_api_key(Some("tmdb-key".into())), + Upstreams::new(prowlarr_url, "http://127.0.0.1:1".into()) + .with_prowlarr_api_key(prowlarr_key) + .with_tmdb_url(tmdb_url) + .with_tmdb_api_key(tmdb_key), ) .expect("state") .with_database(database); @@ -985,6 +1038,94 @@ mod tests { assert_eq!(response["tmdb"][0]["title"], "1917"); } + /// Issue #168: an unreachable TMDB must not cost the operator the + /// library half. The request succeeds, the tmdb group goes empty, and + /// `tmdb_status` says the upstream — not a clean "nothing found". + #[tokio::test] + async fn an_unreachable_tmdb_still_returns_the_library_half() { + // Port 1 is closed; TMDB is configured but cannot be reached. + let (dir, state, base) = application_with( + "http://127.0.0.1:1".into(), + Some("tmdb-key".into()), + Some("prowlarr-key".into()), + "http://127.0.0.1:1".into(), + ) + .await; + sqlx::query("INSERT INTO movies (tmdb_id, title, year, original_language, root_id) VALUES (438631, 'Dune', 2021, 'en', 2)") + .execute(state.database().expect("database").pool()).await.expect("movie"); + + let response = reqwest::get(format!("{base}/api/search?q=Dune")) + .await + .expect("search"); + assert_eq!(response.status(), 200); + let body: serde_json::Value = response.json().await.expect("json"); + assert_eq!(body["library"][0]["kind"], "movie"); + assert_eq!(body["library"][0]["title"], "Dune"); + assert!(body["tmdb"].as_array().expect("tmdb").is_empty()); + assert_eq!(body["tmdb_status"], "unavailable"); + drop(dir); + } + + /// Issue #168: "TMDB answered and found nothing" must stay + /// distinguishable from "TMDB could not be reached" — both leave the + /// tmdb group empty, only `tmdb_status` tells them apart. + #[tokio::test] + async fn an_empty_tmdb_answer_is_not_an_unavailable_tmdb() { + let tmdb = MockServer::start().await; + let prowlarr = MockServer::start().await; + for endpoint in ["/search/movie", "/search/tv"] { + Mock::given(method("GET")) + .and(path(endpoint)) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"results":[]})), + ) + .mount(&tmdb) + .await; + } + let (_dir, _state, base) = application(&tmdb, &prowlarr).await; + + let body: serde_json::Value = reqwest::get(format!("{base}/api/search?q=nonexistent")) + .await + .expect("search") + .json() + .await + .expect("json"); + assert_eq!(body["library"].as_array().expect("library").len(), 0); + assert_eq!(body["tmdb"].as_array().expect("tmdb").len(), 0); + assert_eq!(body["tmdb_status"], "ok"); + } + + /// Issue #168: a Prowlarr fault names Prowlarr, never the database. + #[tokio::test] + async fn a_prowlarr_fault_names_prowlarr() { + let tmdb = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/movie/693134")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id":693_134, "imdb_id":"tt15239678", "title":"Dune Part Two", + "original_title":"Dune Part Two", "original_language":"en", "status":"Released" + }))) + .mount(&tmdb) + .await; + // No Prowlarr API key configured: the client cannot even be built. + let (_dir, state, base) = application_with( + tmdb.uri(), + Some("tmdb-key".into()), + None, + "http://127.0.0.1:1".into(), + ) + .await; + sqlx::query("INSERT INTO movies (tmdb_id, title, year, original_language, root_id) VALUES (693134, 'Dune Part Two', 2024, 'en', 2)") + .execute(state.database().expect("database").pool()).await.expect("movie"); + + let response = reqwest::get(format!("{base}/api/releases?movie_id=1")) + .await + .expect("releases"); + assert_eq!(response.status(), 503); + let body: serde_json::Value = response.json().await.expect("json"); + assert_eq!(body["error"], "prowlarr unavailable"); + } + /// §9.2: a pasted `tt` id resolves a series exactly as a TMDB id does — /// a hit on TMDB and, when tracked, in the library set too. #[tokio::test] diff --git a/web/src/main.ts b/web/src/main.ts index b1d6a33..509bfc6 100644 --- a/web/src/main.ts +++ b/web/src/main.ts @@ -576,11 +576,21 @@ function searchMain( (hit): hit is LibrarySeriesHit => hit.kind === "series", ); const inLibrary = new Set([...libraryMovies, ...librarySeries].map((title) => title.tmdb_id)); + // Issue #168: an unreachable TMDB degrades only its own group — the + // operator sees the library half plus why TMDB is missing, and can tell + // that apart from a clean "nothing found". + const tmdbDown = response.tmdb_status === "unavailable"; if (response.library.length === 0 && response.tmdb.length === 0) { - setStatus("no matches in library or on tmdb"); + setStatus( + tmdbDown ? "no matches in library — tmdb unavailable" : "no matches in library or on tmdb", + ); return; } - setStatus(null); + if (tmdbDown) { + setStatus("tmdb unavailable — showing library matches only", "fault"); + } else { + setStatus(null); + } if (response.library.length > 0) { refs.groups.library.section.hidden = false; diff --git a/web/src/search.ts b/web/src/search.ts index 896454c..c5082f7 100644 --- a/web/src/search.ts +++ b/web/src/search.ts @@ -66,10 +66,18 @@ export interface TmdbSeries { export type TmdbResult = TmdbMovie | TmdbSeries; +/** + * Why the tmdb group is what it is. "ok" with an empty tmdb array means + * TMDB answered and found nothing; "unavailable" means TMDB could not be + * reached — the library results still came back (issue #168). + */ +export type TmdbStatus = "ok" | "unavailable"; + export interface SearchResponse { kind: SearchInputKind; library: LibraryResult[]; tmdb: TmdbResult[]; + tmdb_status: TmdbStatus; manual: string | null; }