Merge #221: give an expired candidate a typed error

Closes #221
This commit is contained in:
Miguel Palhas
2026-08-25 06:30:34 +01:00
5 changed files with 100 additions and 23 deletions
+47 -16
View File
@@ -145,6 +145,11 @@ pub struct Accepted {
#[derive(Debug, Clone, Serialize, ToSchema)] #[derive(Debug, Clone, Serialize, ToSchema)]
pub struct ErrorBody { pub struct ErrorBody {
pub error: String, pub error: String,
/// A machine-readable discriminant, set only where a client needs to
/// branch on the failure rather than display it (issue #221). `None`
/// everywhere else — the message is for the operator, not the client.
#[serde(skip_serializing_if = "Option::is_none")]
pub code: Option<String>,
} }
#[derive(Debug)] #[derive(Debug)]
@@ -166,6 +171,12 @@ pub enum ApiError {
/// four engines are configurable at once and an unnamed failure is /// four engines are configurable at once and an unnamed failure is
/// unactionable. /// unactionable.
SubtitleUpstream(String), SubtitleUpstream(String),
/// A grab named a `candidate_id` the provider no longer recognises
/// (issue #221): a search's results outlive the search itself only in
/// the client's memory, and the provider can expire one at will. Kept
/// apart from [`Self::SubtitleUpstream`] so the panel can offer "search
/// again" from a `code`, not from matching the message text.
SubtitleCandidateExpired,
/// The §9.6 chip outcome: the title exists upstream but has no trailer. /// The §9.6 chip outcome: the title exists upstream but has no trailer.
/// Ordinary, so it must stay distinguishable from an upstream failure. /// Ordinary, so it must stay distinguishable from an upstream failure.
NoTrailer, NoTrailer,
@@ -186,41 +197,61 @@ pub enum ApiError {
impl IntoResponse for ApiError { impl IntoResponse for ApiError {
fn into_response(self) -> Response { fn into_response(self) -> Response {
let (status, error) = match self { let (status, error, code) = match self {
Self::NotFound => (StatusCode::NOT_FOUND, "movie not found".to_string()), Self::NotFound => (StatusCode::NOT_FOUND, "movie not found".to_string(), None),
Self::SeriesNotFound => (StatusCode::NOT_FOUND, "series not found".to_string()), Self::SeriesNotFound => (StatusCode::NOT_FOUND, "series not found".to_string(), None),
Self::SeasonNotFound => (StatusCode::NOT_FOUND, "season not found".to_string()), Self::SeasonNotFound => (StatusCode::NOT_FOUND, "season not found".to_string(), None),
Self::EpisodeNotFound => (StatusCode::NOT_FOUND, "episode not found".to_string()), Self::EpisodeNotFound => (StatusCode::NOT_FOUND, "episode not found".to_string(), None),
Self::OwnerNotFound => (StatusCode::NOT_FOUND, "owner not found".to_string()), Self::OwnerNotFound => (StatusCode::NOT_FOUND, "owner not found".to_string(), None),
Self::PolicyNotFound => (StatusCode::NOT_FOUND, "policy not found".to_string()), Self::PolicyNotFound => (StatusCode::NOT_FOUND, "policy not found".to_string(), None),
Self::RootNotFound => (StatusCode::NOT_FOUND, "root not found".to_string()), Self::RootNotFound => (StatusCode::NOT_FOUND, "root not found".to_string(), None),
Self::MediaFileNotFound => (StatusCode::NOT_FOUND, "media file not found".to_string()), Self::MediaFileNotFound => (
Self::SubtitleNotFound => (StatusCode::NOT_FOUND, "subtitle not found".to_string()), StatusCode::NOT_FOUND,
Self::SubtitleUpstream(error) => (StatusCode::SERVICE_UNAVAILABLE, error), "media file not found".to_string(),
Self::NoTrailer => (StatusCode::NOT_FOUND, "no trailer".to_string()), None,
Self::Conflict(error) => (StatusCode::CONFLICT, error), ),
Self::Invalid(error) => (StatusCode::UNPROCESSABLE_ENTITY, error), Self::SubtitleNotFound => (
StatusCode::NOT_FOUND,
"subtitle not found".to_string(),
None,
),
Self::SubtitleUpstream(error) => (StatusCode::SERVICE_UNAVAILABLE, error, None),
Self::SubtitleCandidateExpired => (
StatusCode::NOT_FOUND,
"candidate no longer exists — search again".to_string(),
Some("candidate_expired".to_string()),
),
Self::NoTrailer => (StatusCode::NOT_FOUND, "no trailer".to_string(), None),
Self::Conflict(error) => (StatusCode::CONFLICT, error, None),
Self::Invalid(error) => (StatusCode::UNPROCESSABLE_ENTITY, error, None),
Self::Unavailable => ( Self::Unavailable => (
StatusCode::SERVICE_UNAVAILABLE, StatusCode::SERVICE_UNAVAILABLE,
"database unavailable".into(), "database unavailable".into(),
None,
), ),
Self::Upstream(name) => ( Self::Upstream(name) => (
StatusCode::SERVICE_UNAVAILABLE, StatusCode::SERVICE_UNAVAILABLE,
format!("{name} unavailable"), format!("{name} unavailable"),
None,
), ),
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(),
None,
)
} }
Self::Filesystem(error) => { Self::Filesystem(error) => {
tracing::error!(%error, "API filesystem error"); tracing::error!(%error, "API filesystem error");
( (
StatusCode::INTERNAL_SERVER_ERROR, StatusCode::INTERNAL_SERVER_ERROR,
format!("files not removed: {error}"), format!("files not removed: {error}"),
None,
) )
} }
}; };
(status, Json(ErrorBody { error })).into_response() (status, Json(ErrorBody { error, code })).into_response()
} }
} }
+26 -1
View File
@@ -968,7 +968,10 @@ pub async fn grab(
let fetched = provider let fetched = provider
.download(&CandidateId::new(input.candidate_id.clone())) .download(&CandidateId::new(input.candidate_id.clone()))
.await .await
.map_err(|error| ApiError::SubtitleUpstream(error.to_string()))?; .map_err(|error| match error {
arr_subs::Error::NotFound { .. } => ApiError::SubtitleCandidateExpired,
other => ApiError::SubtitleUpstream(other.to_string()),
})?;
let text = srt_text(&fetched)?; let text = srt_text(&fetched)?;
write_sidecar(&destination, &text).await?; write_sidecar(&destination, &text).await?;
@@ -2307,6 +2310,28 @@ mod tests {
.exists()); .exists());
} }
/// A candidate id does not outlive the search that produced it (issue
/// #221): the panel gets a typed `code` to switch on, not a string it has
/// to pattern-match against a provider-chosen message.
#[tokio::test]
async fn a_grab_of_an_expired_candidate_is_a_typed_404() {
let fixture = application(vec![Arc::new(DeadProvider)], vec![]).await;
let (status, body) = grab(
&fixture,
serde_json::json!({
"provider": "dead",
"candidate_id": "stale",
"language": "pt-PT"
}),
)
.await;
assert_eq!(status, StatusCode::NOT_FOUND, "{body}");
assert_eq!(body["code"], "candidate_expired");
assert!(fixture.subtitle_rows().await.is_empty());
}
#[tokio::test] #[tokio::test]
async fn a_grab_naming_an_unconfigured_provider_is_a_422() { async fn a_grab_naming_an_unconfigured_provider_is_a_422() {
let fixture = stub_application().await; let fixture = stub_application().await;
+4 -3
View File
@@ -1320,9 +1320,10 @@ function subtitleSection(status: SubtitleStatus, refresh: () => void): SubtitleS
say(note, "fetching, syncing, writing…"); say(note, "fetching, syncing, writing…");
const outcome = await grabSubtitle(mediaFileId, candidate); const outcome = await grabSubtitle(mediaFileId, candidate);
if (outcome.kind === "error") { if (outcome.kind === "error") {
const stale = /not found/i.test(outcome.detail) const stale =
? " — search again, a candidate id does not outlive its search" outcome.code === "candidate_expired"
: ""; ? " — search again, a candidate id does not outlive its search"
: "";
say(note, `grab failed — ${outcome.detail}${stale}`, "fault"); say(note, `grab failed — ${outcome.detail}${stale}`, "fault");
return false; return false;
} }
+15
View File
@@ -463,3 +463,18 @@ export async function errorDetail(response: Response): Promise<string> {
return `http ${response.status}`; return `http ${response.status}`;
} }
} }
/**
* Like {@link errorDetail}, but also surfaces `ErrorBody.code` — the
* machine-readable discriminant a client switches on instead of matching the
* message text (issue #221). `code` is `undefined` on every error that has
* none, which is most of them.
*/
export async function errorBody(response: Response): Promise<{ detail: string; code?: string }> {
try {
const body = (await response.json()) as { error?: string; code?: string };
return { detail: body.error ?? `http ${response.status}`, code: body.code };
} catch {
return { detail: `http ${response.status}` };
}
}
+8 -3
View File
@@ -2,7 +2,7 @@
// schemas — same reasoning as search.ts: the generated client (src/api/) is // schemas — same reasoning as search.ts: the generated client (src/api/) is
// uncommitted, so CI's tsc cannot see it. // uncommitted, so CI's tsc cannot see it.
import { errorDetail } from "./releases"; import { errorBody, errorDetail } from "./releases";
/** One subtitle arr knows about, as `/subtitles` and `/subtitles/status` render it. */ /** One subtitle arr knows about, as `/subtitles` and `/subtitles/status` render it. */
export interface Subtitle { export interface Subtitle {
@@ -201,12 +201,16 @@ export async function searchSubtitles(
export type SubtitleWriteOutcome = export type SubtitleWriteOutcome =
| { kind: "done"; subtitle: Subtitle } | { kind: "done"; subtitle: Subtitle }
| { kind: "error"; detail: string }; | { kind: "error"; detail: string; code?: string };
/** /**
* One click: the daemon fetches the candidate, runs `alass` over it and * One click: the daemon fetches the candidate, runs `alass` over it and
* writes the sidecar (§15). The candidate's own flags travel with it — * writes the sidecar (§15). The candidate's own flags travel with it —
* nothing on the server remembers a search. * nothing on the server remembers a search.
*
* A grab can fail because the candidate itself expired between the search
* and the click (issue #221) — the server reports that as `code:
* "candidate_expired"`, not by wording the message a particular way.
*/ */
export async function grabSubtitle( export async function grabSubtitle(
mediaFileId: number, mediaFileId: number,
@@ -225,7 +229,8 @@ export async function grabSubtitle(
}), }),
}); });
if (!response.ok) { if (!response.ok) {
return { kind: "error", detail: await errorDetail(response) }; const { detail, code } = await errorBody(response);
return { kind: "error", detail, code };
} }
return { kind: "done", subtitle: (await response.json()) as Subtitle }; return { kind: "done", subtitle: (await response.json()) as Subtitle };
} catch { } catch {