fix(api): give an expired subtitle candidate a typed error
A grab naming a stale candidate_id now fails as ApiError::SubtitleCandidateExpired (404, code candidate_expired) instead of the generic upstream 503 string the panel had to pattern-match for 'not found'.
This commit is contained in:
@@ -145,6 +145,11 @@ pub struct Accepted {
|
||||
#[derive(Debug, Clone, Serialize, ToSchema)]
|
||||
pub struct ErrorBody {
|
||||
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)]
|
||||
@@ -166,6 +171,12 @@ pub enum ApiError {
|
||||
/// four engines are configurable at once and an unnamed failure is
|
||||
/// unactionable.
|
||||
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.
|
||||
/// Ordinary, so it must stay distinguishable from an upstream failure.
|
||||
NoTrailer,
|
||||
@@ -186,41 +197,61 @@ pub enum ApiError {
|
||||
|
||||
impl IntoResponse for ApiError {
|
||||
fn into_response(self) -> Response {
|
||||
let (status, error) = match self {
|
||||
Self::NotFound => (StatusCode::NOT_FOUND, "movie not found".to_string()),
|
||||
Self::SeriesNotFound => (StatusCode::NOT_FOUND, "series not found".to_string()),
|
||||
Self::SeasonNotFound => (StatusCode::NOT_FOUND, "season not found".to_string()),
|
||||
Self::EpisodeNotFound => (StatusCode::NOT_FOUND, "episode not found".to_string()),
|
||||
Self::OwnerNotFound => (StatusCode::NOT_FOUND, "owner not found".to_string()),
|
||||
Self::PolicyNotFound => (StatusCode::NOT_FOUND, "policy not found".to_string()),
|
||||
Self::RootNotFound => (StatusCode::NOT_FOUND, "root not found".to_string()),
|
||||
Self::MediaFileNotFound => (StatusCode::NOT_FOUND, "media file not found".to_string()),
|
||||
Self::SubtitleNotFound => (StatusCode::NOT_FOUND, "subtitle not found".to_string()),
|
||||
Self::SubtitleUpstream(error) => (StatusCode::SERVICE_UNAVAILABLE, error),
|
||||
Self::NoTrailer => (StatusCode::NOT_FOUND, "no trailer".to_string()),
|
||||
Self::Conflict(error) => (StatusCode::CONFLICT, error),
|
||||
Self::Invalid(error) => (StatusCode::UNPROCESSABLE_ENTITY, error),
|
||||
let (status, error, code) = match self {
|
||||
Self::NotFound => (StatusCode::NOT_FOUND, "movie not found".to_string(), None),
|
||||
Self::SeriesNotFound => (StatusCode::NOT_FOUND, "series not found".to_string(), None),
|
||||
Self::SeasonNotFound => (StatusCode::NOT_FOUND, "season not found".to_string(), None),
|
||||
Self::EpisodeNotFound => (StatusCode::NOT_FOUND, "episode not found".to_string(), None),
|
||||
Self::OwnerNotFound => (StatusCode::NOT_FOUND, "owner not found".to_string(), None),
|
||||
Self::PolicyNotFound => (StatusCode::NOT_FOUND, "policy not found".to_string(), None),
|
||||
Self::RootNotFound => (StatusCode::NOT_FOUND, "root not found".to_string(), None),
|
||||
Self::MediaFileNotFound => (
|
||||
StatusCode::NOT_FOUND,
|
||||
"media file not found".to_string(),
|
||||
None,
|
||||
),
|
||||
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 => (
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"database unavailable".into(),
|
||||
None,
|
||||
),
|
||||
Self::Upstream(name) => (
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
format!("{name} unavailable"),
|
||||
None,
|
||||
),
|
||||
Self::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) => {
|
||||
tracing::error!(%error, "API filesystem error");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("files not removed: {error}"),
|
||||
None,
|
||||
)
|
||||
}
|
||||
};
|
||||
(status, Json(ErrorBody { error })).into_response()
|
||||
(status, Json(ErrorBody { error, code })).into_response()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -968,7 +968,10 @@ pub async fn grab(
|
||||
let fetched = provider
|
||||
.download(&CandidateId::new(input.candidate_id.clone()))
|
||||
.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)?;
|
||||
write_sidecar(&destination, &text).await?;
|
||||
|
||||
@@ -2301,6 +2304,28 @@ mod tests {
|
||||
.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]
|
||||
async fn a_grab_naming_an_unconfigured_provider_is_a_422() {
|
||||
let fixture = stub_application().await;
|
||||
|
||||
+2
-1
@@ -1320,7 +1320,8 @@ function subtitleSection(status: SubtitleStatus, refresh: () => void): SubtitleS
|
||||
say(note, "fetching, syncing, writing…");
|
||||
const outcome = await grabSubtitle(mediaFileId, candidate);
|
||||
if (outcome.kind === "error") {
|
||||
const stale = /not found/i.test(outcome.detail)
|
||||
const stale =
|
||||
outcome.code === "candidate_expired"
|
||||
? " — search again, a candidate id does not outlive its search"
|
||||
: "";
|
||||
say(note, `grab failed — ${outcome.detail}${stale}`, "fault");
|
||||
|
||||
@@ -463,3 +463,18 @@ export async function errorDetail(response: Response): Promise<string> {
|
||||
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}` };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
// schemas — same reasoning as search.ts: the generated client (src/api/) is
|
||||
// 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. */
|
||||
export interface Subtitle {
|
||||
@@ -201,12 +201,16 @@ export async function searchSubtitles(
|
||||
|
||||
export type SubtitleWriteOutcome =
|
||||
| { 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
|
||||
* writes the sidecar (§15). The candidate's own flags travel with it —
|
||||
* 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(
|
||||
mediaFileId: number,
|
||||
@@ -225,7 +229,8 @@ export async function grabSubtitle(
|
||||
}),
|
||||
});
|
||||
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 };
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user