fix panic in risuto-server, leftover after the chrono fix

Ekleog · Jan 22, 2023 · 9c23cc8 · 9c23cc8
1 parent 70abd09
commit 9c23cc8
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 26 deletions.
diff --git a/risuto-api/src/error.rs b/risuto-api/src/error.rs
@@ -31,6 +31,9 @@ pub enum Error {
 
     #[error("Invalid time")]
     InvalidTime(Time),
+
+    #[error("Integer is out of expected range")]
+    IntegerOutOfRange(i64),
 }
 
 impl Error {
@@ -45,6 +48,7 @@ impl Error {
             Error::NullByteInString(_) => StatusCode::BAD_REQUEST,
             Error::InvalidName(_) => StatusCode::BAD_REQUEST,
             Error::InvalidTime(_) => StatusCode::BAD_REQUEST,
+            Error::IntegerOutOfRange(_) => StatusCode::BAD_REQUEST,
         }
     }
 
@@ -87,6 +91,11 @@ impl Error {
                 "type": "invalid-time",
                 "time": t,
             }),
+            Error::IntegerOutOfRange(i) => json!({
+                "message": "integer is outside of acceptable range",
+                "type": "integer-out-of-range",
+                "int": i,
+            }),
         })
         .expect("serializing conflict")
     }
@@ -135,6 +144,13 @@ impl Error {
                             anyhow!("error is about an invalid time but no time was provided")
                         })?,
                 ),
+                "integer-out-of-range" => Error::IntegerOutOfRange(
+                    data.get("int").and_then(|s| s.as_i64()).ok_or_else(|| {
+                        anyhow!(
+                            "error is about an integer out of range but no integer was provided"
+                        )
+                    })?,
+                ),
                 _ => return Err(anyhow!("error contents has unknown type")),
             },
         )

diff --git a/risuto-server/src/db.rs b/risuto-server/src/db.rs
@@ -445,11 +445,11 @@ pub fn users_interested_by<'conn>(
     .map(|r| r.map(|u| UserId(u.user_id)).map_err(anyhow::Error::from))
 }
 
-async fn with_tmp_tasks_table<R, F>(conn: &mut sqlx::PgConnection, f: F) -> anyhow::Result<R>
+async fn with_tmp_tasks_table<R, F>(conn: &mut sqlx::PgConnection, f: F) -> Result<R, Error>
 where
     F: for<'a> FnOnce(
         &'a mut sqlx::PgConnection,
-    ) -> Pin<Box<dyn 'a + Send + Future<Output = anyhow::Result<R>>>>,
+    ) -> Pin<Box<dyn 'a + Send + Future<Output = Result<R, Error>>>>,
 {
     sqlx::query("CREATE TEMPORARY TABLE tmp_tasks (id UUID NOT NULL)")
         .execute(&mut *conn)
@@ -570,11 +570,11 @@ pub async fn search_tasks_for_user(
     conn: &mut sqlx::PgConnection,
     owner: UserId,
     query: &Query,
-) -> anyhow::Result<(Vec<Task>, Vec<Event>)> {
+) -> Result<(Vec<Task>, Vec<Event>), Error> {
     let query::Sql {
         where_clause,
         binds,
-    } = query::to_postgres(&query, 2);
+    } = query::to_postgres(&query, 2)?;
     with_tmp_tasks_table(&mut *conn, |conn| {
         Box::pin(async move {
             let query = format!(
@@ -623,7 +623,7 @@ pub async fn search_tasks_for_user(
 
 async fn fetch_tasks_from_tmp_tasks_table(
     conn: &mut sqlx::PgConnection,
-) -> anyhow::Result<(Vec<Task>, Vec<Event>)> {
+) -> Result<(Vec<Task>, Vec<Event>), Error> {
     let tasks = sqlx::query_as::<_, DbTask>(
         "
             SELECT t.id, t.owner_id, t.date, t.initial_title, t.top_comment_id

diff --git a/risuto-server/src/error.rs b/risuto-server/src/error.rs
@@ -25,6 +25,10 @@ impl Error {
     pub fn invalid_pow() -> Error {
         Error::Api(ApiError::InvalidPow)
     }
+
+    pub fn integer_out_of_range(i: i64) -> Error {
+        Error::Api(ApiError::IntegerOutOfRange(i))
+    }
 }
 
 impl axum::response::IntoResponse for Error {

diff --git a/risuto-server/src/handlers.rs b/risuto-server/src/handlers.rs
@@ -98,11 +98,7 @@ pub async fn search_tasks(
     Json(q): Json<risuto_api::Query>,
 ) -> Result<Json<(Vec<Task>, Vec<Event>)>, Error> {
     q.validate()?;
-    Ok(Json(
-        db::search_tasks_for_user(&mut *conn, user, &q)
-            .await
-            .with_context(|| format!("fetching task list for {:?}", user))?,
-    ))
+    Ok(Json(db::search_tasks_for_user(&mut *conn, user, &q).await?))
 }
 
 pub async fn submit_action(

diff --git a/risuto-server/src/query.rs b/risuto-server/src/query.rs
@@ -1,5 +1,7 @@
 use risuto_api::{Query, Time, TimeQuery, Uuid};
 
+use crate::error::Error;
+
 pub enum Bind {
     Bool(bool),
     Uuid(Uuid),
@@ -25,33 +27,33 @@ impl Sql {
 /// Assumes tables vta (v_tasks_archived), vtd(v_tasks_done), vtt (v_tasks_tags),
 /// vtit (v_tasks_is_tagged), vts (v_tasks_scheduled), vtb (v_tasks_blocked)
 /// and vtx (v_tasks_text) are available
-pub fn to_postgres(q: &Query, first_bind_idx: usize) -> Sql {
+pub fn to_postgres(q: &Query, first_bind_idx: usize) -> Result<Sql, Error> {
     let mut res = Default::default();
-    add_to_postgres(q, first_bind_idx, &mut res);
-    res
+    add_to_postgres(q, first_bind_idx, &mut res)?;
+    Ok(res)
 }
 
-fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) {
+fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) -> Result<(), Error> {
     match q {
         Query::Any(queries) => {
             res.where_clause.push_str("(false");
             for q in queries {
                 res.where_clause.push_str(" OR ");
-                add_to_postgres(q, first_bind_idx, &mut *res);
+                add_to_postgres(q, first_bind_idx, &mut *res)?;
             }
             res.where_clause.push(')');
         }
         Query::All(queries) => {
             res.where_clause.push_str("(true");
             for q in queries {
                 res.where_clause.push_str(" AND ");
-                add_to_postgres(q, first_bind_idx, &mut *res);
+                add_to_postgres(q, first_bind_idx, &mut *res)?;
             }
             res.where_clause.push(')');
         }
         Query::Not(q) => {
             res.where_clause.push_str("NOT ");
-            add_to_postgres(q, first_bind_idx, &mut *res);
+            add_to_postgres(q, first_bind_idx, &mut *res)?;
         }
         Query::Archived(true) => {
             res.where_clause.push_str("vta.archived = true");
@@ -86,19 +88,19 @@ fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) {
                 .push_str("(vtit.has_tag = false OR vtit.has_tag IS NULL)");
         }
         Query::ScheduledForBefore(date) => {
-            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date));
+            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?);
             res.where_clause.push_str(&format!("(vts.time <= ${idx})"));
         }
         Query::ScheduledForAfter(date) => {
-            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date));
+            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?);
             res.where_clause.push_str(&format!("(vts.time >= ${idx})"));
         }
         Query::BlockedUntilAtMost(date) => {
-            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date));
+            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?);
             res.where_clause.push_str(&format!("(vtb.time <= ${idx})"));
         }
         Query::BlockedUntilAtLeast(date) => {
-            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date));
+            let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?);
             res.where_clause.push_str(&format!("(vtb.time >= ${idx})"));
         }
         Query::Phrase(t) => {
@@ -107,10 +109,11 @@ fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) {
                 .push_str(&format!("(vtx.text @@ phraseto_tsquery(${idx}))"));
         }
     }
+    Ok(())
 }
 
-fn timeq_to_bind(q: &TimeQuery) -> Bind {
-    Bind::Time(match q {
+fn timeq_to_bind(q: &TimeQuery) -> Result<Bind, Error> {
+    Ok(Bind::Time(match q {
         TimeQuery::Absolute(t) => *t,
         TimeQuery::DayRelative {
             timezone,
@@ -121,16 +124,16 @@ fn timeq_to_bind(q: &TimeQuery) -> Bind {
             let date = match *day_offset > 0 {
                 true => date
                     .checked_add_days(chrono::naive::Days::new(*day_offset as u64))
-                    .expect("failed adding days"),
+                    .ok_or_else(|| Error::integer_out_of_range(*day_offset))?,
                 false => date
                     .checked_sub_days(chrono::naive::Days::new((-day_offset) as u64))
-                    .expect("failed subtracting days"),
+                    .ok_or_else(|| Error::integer_out_of_range(*day_offset))?,
             };
             date.and_hms_opt(0, 0, 0)
                 .expect("naive_date and hms 000 failed")
                 .and_local_timezone(timezone.clone())
                 .unwrap()
                 .with_timezone(&chrono::Utc)
         }
-    })
+    }))
 }