fix(schedules): use typed error for pause/resume 404 discrimination
Replace brittle msg.contains("not found") string matching in
post_schedule_pause / post_schedule_resume with a typed
ScheduleNotFoundOrCancelled error that handlers downcast on directly.
pause() and resume() now return Err(ScheduleNotFoundOrCancelled(id).into())
instead of bail!("schedule {id} not found or is cancelled"); handlers call
e.downcast_ref::<ScheduleNotFoundOrCancelled>().is_some() for the 404 branch,
making the discrimination stable even if the error message wording changes.
This commit is contained in:
parent
2bfa5bc1a8
commit
dbd4b7a15c
2 changed files with 28 additions and 12 deletions
|
|
@ -13,6 +13,8 @@ use axum::{
|
||||||
|
|
||||||
use problem_details::ProblemDetails;
|
use problem_details::ProblemDetails;
|
||||||
|
|
||||||
|
use crate::scheduled_prompts::ScheduleNotFoundOrCancelled;
|
||||||
|
|
||||||
use super::{AppState, error_problem, error_response};
|
use super::{AppState, error_problem, error_response};
|
||||||
|
|
||||||
/// `GET /api/schedules` — snapshot of every schedule for the
|
/// `GET /api/schedules` — snapshot of every schedule for the
|
||||||
|
|
@ -241,11 +243,10 @@ pub(super) async fn post_schedule_pause(
|
||||||
(StatusCode::OK, "ok").into_response()
|
(StatusCode::OK, "ok").into_response()
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let msg = format!("{e:#}");
|
if e.downcast_ref::<ScheduleNotFoundOrCancelled>().is_some() {
|
||||||
if msg.contains("not found") || msg.contains("cancelled") {
|
(StatusCode::NOT_FOUND, format!("{e}")).into_response()
|
||||||
(StatusCode::NOT_FOUND, msg).into_response()
|
|
||||||
} else {
|
} else {
|
||||||
error_response(&format!("pause schedule {id}: {msg}"))
|
error_response(&format!("pause schedule {id}: {e:#}"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -264,11 +265,10 @@ pub(super) async fn post_schedule_resume(
|
||||||
(StatusCode::OK, "ok").into_response()
|
(StatusCode::OK, "ok").into_response()
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let msg = format!("{e:#}");
|
if e.downcast_ref::<ScheduleNotFoundOrCancelled>().is_some() {
|
||||||
if msg.contains("not found") || msg.contains("cancelled") {
|
(StatusCode::NOT_FOUND, format!("{e}")).into_response()
|
||||||
(StatusCode::NOT_FOUND, msg).into_response()
|
|
||||||
} else {
|
} else {
|
||||||
error_response(&format!("resume schedule {id}: {msg}"))
|
error_response(&format!("resume schedule {id}: {e:#}"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,20 @@ use anyhow::{Context, Result, bail};
|
||||||
use rusqlite::{Connection, OptionalExtension, params};
|
use rusqlite::{Connection, OptionalExtension, params};
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
|
||||||
|
/// Typed error returned by [`ScheduledPrompts::pause`] and
|
||||||
|
/// [`ScheduledPrompts::resume`] when the target row does not exist or
|
||||||
|
/// is already cancelled. Handlers downcast on this type to emit 404
|
||||||
|
/// rather than 500, avoiding brittle string-matching on the message.
|
||||||
|
#[derive(Debug)]
|
||||||
|
pub struct ScheduleNotFoundOrCancelled(pub i64);
|
||||||
|
|
||||||
|
impl std::fmt::Display for ScheduleNotFoundOrCancelled {
|
||||||
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
|
write!(f, "schedule {} not found or is cancelled", self.0)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
impl std::error::Error for ScheduleNotFoundOrCancelled {}
|
||||||
|
|
||||||
const SCHEMA: &str = r"
|
const SCHEMA: &str = r"
|
||||||
CREATE TABLE IF NOT EXISTS scheduled_prompts (
|
CREATE TABLE IF NOT EXISTS scheduled_prompts (
|
||||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||||
|
|
@ -577,7 +591,8 @@ impl ScheduledPrompts {
|
||||||
/// `next_fire_at_unix` is preserved so the first resume fires at
|
/// `next_fire_at_unix` is preserved so the first resume fires at
|
||||||
/// the next intended cadence instant (no catch-up needed — a
|
/// the next intended cadence instant (no catch-up needed — a
|
||||||
/// paused schedule simply slips its upcoming fire). Returns
|
/// paused schedule simply slips its upcoming fire). Returns
|
||||||
/// `Err` when the schedule is cancelled or does not exist.
|
/// `Err(ScheduleNotFoundOrCancelled)` when the schedule is
|
||||||
|
/// cancelled or does not exist (handlers downcast to emit 404).
|
||||||
pub fn pause(&self, id: i64) -> Result<()> {
|
pub fn pause(&self, id: i64) -> Result<()> {
|
||||||
let conn = self.conn.lock().unwrap();
|
let conn = self.conn.lock().unwrap();
|
||||||
let now = now_unix();
|
let now = now_unix();
|
||||||
|
|
@ -588,13 +603,14 @@ impl ScheduledPrompts {
|
||||||
params![now, id],
|
params![now, id],
|
||||||
)?;
|
)?;
|
||||||
if n == 0 {
|
if n == 0 {
|
||||||
bail!("schedule {id} not found or is cancelled");
|
return Err(ScheduleNotFoundOrCancelled(id).into());
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Resume a paused schedule. Idempotent; no-op on an active row.
|
/// Resume a paused schedule. Idempotent; no-op on an active row.
|
||||||
/// Returns `Err` when the schedule is cancelled or does not exist.
|
/// Returns `Err(ScheduleNotFoundOrCancelled)` when the schedule is
|
||||||
|
/// cancelled or does not exist (handlers downcast to emit 404).
|
||||||
pub fn resume(&self, id: i64) -> Result<()> {
|
pub fn resume(&self, id: i64) -> Result<()> {
|
||||||
let conn = self.conn.lock().unwrap();
|
let conn = self.conn.lock().unwrap();
|
||||||
let n = conn.execute(
|
let n = conn.execute(
|
||||||
|
|
@ -604,7 +620,7 @@ impl ScheduledPrompts {
|
||||||
params![id],
|
params![id],
|
||||||
)?;
|
)?;
|
||||||
if n == 0 {
|
if n == 0 {
|
||||||
bail!("schedule {id} not found or is cancelled");
|
return Err(ScheduleNotFoundOrCancelled(id).into());
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue