fix(api): search survives a down TMDB and errors name the upstream
This commit is contained in:
@@ -159,7 +159,12 @@ pub enum ApiError {
|
|||||||
NoTrailer,
|
NoTrailer,
|
||||||
Conflict(String),
|
Conflict(String),
|
||||||
Invalid(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,
|
Unavailable,
|
||||||
|
/// An upstream service could not be reached, is misconfigured or is
|
||||||
|
/// unconfigured. The payload names it.
|
||||||
|
Upstream(&'static str),
|
||||||
Database(String),
|
Database(String),
|
||||||
/// A library delete that could not touch the disk. Named separately from
|
/// 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
|
/// [`Self::Database`] because the row is still there and a retry is the
|
||||||
@@ -184,6 +189,10 @@ impl IntoResponse for ApiError {
|
|||||||
StatusCode::SERVICE_UNAVAILABLE,
|
StatusCode::SERVICE_UNAVAILABLE,
|
||||||
"database unavailable".into(),
|
"database unavailable".into(),
|
||||||
),
|
),
|
||||||
|
Self::Upstream(name) => (
|
||||||
|
StatusCode::SERVICE_UNAVAILABLE,
|
||||||
|
format!("{name} unavailable"),
|
||||||
|
),
|
||||||
Self::Database(error) => {
|
Self::Database(error) => {
|
||||||
tracing::error!(%error, "API database error");
|
tracing::error!(%error, "API database error");
|
||||||
(StatusCode::INTERNAL_SERVER_ERROR, "database error".into())
|
(StatusCode::INTERNAL_SERVER_ERROR, "database error".into())
|
||||||
@@ -554,7 +563,7 @@ pub async fn search(
|
|||||||
}
|
}
|
||||||
state
|
state
|
||||||
.send_movie_command(MovieCommand::Search { movie_id: id })
|
.send_movie_command(MovieCommand::Search { movie_id: id })
|
||||||
.map_err(|_| ApiError::Unavailable)?;
|
.map_err(|_| ApiError::Upstream("daemon"))?;
|
||||||
Ok((StatusCode::ACCEPTED, Json(Accepted { accepted: true })))
|
Ok((StatusCode::ACCEPTED, Json(Accepted { accepted: true })))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -671,7 +680,7 @@ pub async fn grab(
|
|||||||
movie_id,
|
movie_id,
|
||||||
release_id,
|
release_id,
|
||||||
})
|
})
|
||||||
.map_err(|_| ApiError::Unavailable)?;
|
.map_err(|_| ApiError::Upstream("daemon"))?;
|
||||||
Ok((StatusCode::ACCEPTED, Json(Accepted { accepted: true })))
|
Ok((StatusCode::ACCEPTED, Json(Accepted { accepted: true })))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+154
-13
@@ -36,9 +36,23 @@ pub struct SearchResponse {
|
|||||||
/// In-library hits, grouped first (§9.2): movies and series by title.
|
/// In-library hits, grouped first (§9.2): movies and series by title.
|
||||||
pub library: Vec<LibraryResult>,
|
pub library: Vec<LibraryResult>,
|
||||||
pub tmdb: Vec<TmdbResult>,
|
pub tmdb: Vec<TmdbResult>,
|
||||||
|
/// 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<String>,
|
pub manual: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[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
|
/// One in-library hit. Tagged so a client can branch on what it found
|
||||||
/// without re-deriving it from fields.
|
/// without re-deriving it from fields.
|
||||||
#[derive(Debug, Clone, Serialize, ToSchema)]
|
#[derive(Debug, Clone, Serialize, ToSchema)]
|
||||||
@@ -167,6 +181,7 @@ pub async fn search(
|
|||||||
kind,
|
kind,
|
||||||
library: Vec::new(),
|
library: Vec::new(),
|
||||||
tmdb: Vec::new(),
|
tmdb: Vec::new(),
|
||||||
|
tmdb_status: TmdbStatus::Ok,
|
||||||
manual: Some(input.to_owned()),
|
manual: Some(input.to_owned()),
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
@@ -211,8 +226,27 @@ pub async fn search(
|
|||||||
};
|
};
|
||||||
library.extend(series_rows.into_iter().map(LibraryResult::Series));
|
library.extend(series_rows.into_iter().map(LibraryResult::Series));
|
||||||
|
|
||||||
let tmdb = tmdb_client(&state)?;
|
// A degraded TMDB must not discard the library results already in hand
|
||||||
let results = search_tmdb(&tmdb, kind, input).await?;
|
// (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) {
|
if matches!(kind, SearchInputKind::ImdbId) {
|
||||||
for result in &results {
|
for result in &results {
|
||||||
match result {
|
match result {
|
||||||
@@ -240,6 +274,7 @@ pub async fn search(
|
|||||||
kind,
|
kind,
|
||||||
library,
|
library,
|
||||||
tmdb: results,
|
tmdb: results,
|
||||||
|
tmdb_status,
|
||||||
manual: None,
|
manual: None,
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
@@ -415,7 +450,7 @@ async fn movie_releases(
|
|||||||
let indexers = prowlarr
|
let indexers = prowlarr
|
||||||
.indexers()
|
.indexers()
|
||||||
.await
|
.await
|
||||||
.map_err(|_| ApiError::Unavailable)?;
|
.map_err(|_| ApiError::Upstream("prowlarr"))?;
|
||||||
let blacklist = Blacklist::load(database.pool()).await?;
|
let blacklist = Blacklist::load(database.pool()).await?;
|
||||||
let policy = loaded.policy;
|
let policy = loaded.policy;
|
||||||
let overrides = loaded.overrides;
|
let overrides = loaded.overrides;
|
||||||
@@ -512,7 +547,7 @@ async fn episode_releases(
|
|||||||
let indexers = prowlarr
|
let indexers = prowlarr
|
||||||
.indexers()
|
.indexers()
|
||||||
.await
|
.await
|
||||||
.map_err(|_| ApiError::Unavailable)?;
|
.map_err(|_| ApiError::Upstream("prowlarr"))?;
|
||||||
let blacklist = Blacklist::load(database.pool()).await?;
|
let blacklist = Blacklist::load(database.pool()).await?;
|
||||||
|
|
||||||
let mut classified = Vec::new();
|
let mut classified = Vec::new();
|
||||||
@@ -634,8 +669,9 @@ fn prowlarr_client(state: &AppState) -> Result<ProwlarrClient, ApiError> {
|
|||||||
let api_key = upstreams
|
let api_key = upstreams
|
||||||
.prowlarr_api_key
|
.prowlarr_api_key
|
||||||
.clone()
|
.clone()
|
||||||
.ok_or(ApiError::Unavailable)?;
|
.ok_or(ApiError::Upstream("prowlarr"))?;
|
||||||
ProwlarrClient::new(upstreams.prowlarr_url.clone(), api_key).map_err(|_| ApiError::Unavailable)
|
ProwlarrClient::new(upstreams.prowlarr_url.clone(), api_key)
|
||||||
|
.map_err(|_| ApiError::Upstream("prowlarr"))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn tmdb_client(state: &AppState) -> Result<arr_meta::TmdbClient, ApiError> {
|
pub(crate) fn tmdb_client(state: &AppState) -> Result<arr_meta::TmdbClient, ApiError> {
|
||||||
@@ -643,17 +679,17 @@ pub(crate) fn tmdb_client(state: &AppState) -> Result<arr_meta::TmdbClient, ApiE
|
|||||||
let key = upstreams
|
let key = upstreams
|
||||||
.tmdb_api_key
|
.tmdb_api_key
|
||||||
.clone()
|
.clone()
|
||||||
.ok_or(ApiError::Unavailable)?;
|
.ok_or(ApiError::Upstream("tmdb"))?;
|
||||||
arr_meta::TmdbClient::builder(key)
|
arr_meta::TmdbClient::builder(key)
|
||||||
.base_url(&upstreams.tmdb_url)
|
.base_url(&upstreams.tmdb_url)
|
||||||
.build()
|
.build()
|
||||||
.map_err(|_| ApiError::Unavailable)
|
.map_err(|_| ApiError::Upstream("tmdb"))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn upstream_error(error: &arr_meta::Error) -> ApiError {
|
pub(crate) fn upstream_error(error: &arr_meta::Error) -> ApiError {
|
||||||
match error {
|
match error {
|
||||||
arr_meta::Error::NotFound { .. } => ApiError::NotFound,
|
arr_meta::Error::NotFound { .. } => ApiError::NotFound,
|
||||||
_ => ApiError::Unavailable,
|
_ => ApiError::Upstream("tmdb"),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -781,6 +817,23 @@ mod tests {
|
|||||||
async fn application(
|
async fn application(
|
||||||
tmdb: &MockServer,
|
tmdb: &MockServer,
|
||||||
prowlarr: &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<String>,
|
||||||
|
prowlarr_key: Option<String>,
|
||||||
|
prowlarr_url: String,
|
||||||
) -> (tempfile::TempDir, AppState, String) {
|
) -> (tempfile::TempDir, AppState, String) {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
let database = arr_db::Db::connect(dir.path().join("arr.db"))
|
let database = arr_db::Db::connect(dir.path().join("arr.db"))
|
||||||
@@ -788,10 +841,10 @@ mod tests {
|
|||||||
.expect("database");
|
.expect("database");
|
||||||
database.migrate().await.expect("migrate");
|
database.migrate().await.expect("migrate");
|
||||||
let state = AppState::new(
|
let state = AppState::new(
|
||||||
Upstreams::new(prowlarr.uri(), "http://127.0.0.1:1".into())
|
Upstreams::new(prowlarr_url, "http://127.0.0.1:1".into())
|
||||||
.with_prowlarr_api_key(Some("prowlarr-key".into()))
|
.with_prowlarr_api_key(prowlarr_key)
|
||||||
.with_tmdb_url(tmdb.uri())
|
.with_tmdb_url(tmdb_url)
|
||||||
.with_tmdb_api_key(Some("tmdb-key".into())),
|
.with_tmdb_api_key(tmdb_key),
|
||||||
)
|
)
|
||||||
.expect("state")
|
.expect("state")
|
||||||
.with_database(database);
|
.with_database(database);
|
||||||
@@ -985,6 +1038,94 @@ mod tests {
|
|||||||
assert_eq!(response["tmdb"][0]["title"], "1917");
|
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 —
|
/// §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.
|
/// a hit on TMDB and, when tracked, in the library set too.
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
+12
-2
@@ -576,11 +576,21 @@ function searchMain(
|
|||||||
(hit): hit is LibrarySeriesHit => hit.kind === "series",
|
(hit): hit is LibrarySeriesHit => hit.kind === "series",
|
||||||
);
|
);
|
||||||
const inLibrary = new Set([...libraryMovies, ...librarySeries].map((title) => title.tmdb_id));
|
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) {
|
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;
|
return;
|
||||||
}
|
}
|
||||||
setStatus(null);
|
if (tmdbDown) {
|
||||||
|
setStatus("tmdb unavailable — showing library matches only", "fault");
|
||||||
|
} else {
|
||||||
|
setStatus(null);
|
||||||
|
}
|
||||||
|
|
||||||
if (response.library.length > 0) {
|
if (response.library.length > 0) {
|
||||||
refs.groups.library.section.hidden = false;
|
refs.groups.library.section.hidden = false;
|
||||||
|
|||||||
@@ -66,10 +66,18 @@ export interface TmdbSeries {
|
|||||||
|
|
||||||
export type TmdbResult = TmdbMovie | 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 {
|
export interface SearchResponse {
|
||||||
kind: SearchInputKind;
|
kind: SearchInputKind;
|
||||||
library: LibraryResult[];
|
library: LibraryResult[];
|
||||||
tmdb: TmdbResult[];
|
tmdb: TmdbResult[];
|
||||||
|
tmdb_status: TmdbStatus;
|
||||||
manual: string | null;
|
manual: string | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user