From 5aac6ec1a46a64b810f4695de968a10cab000914 Mon Sep 17 00:00:00 2001
From: Bryan Newbold <bnewbold@robocracy.org>
Date: Tue, 29 Jan 2019 12:50:39 -0800
Subject: better database NotFound error propagation

---
 rust/src/editing_crud.rs | 40 +++++++++++++++++++++++++++++++++++-----
 rust/src/endpoints.rs    | 14 +++++++++++---
 rust/src/errors.rs       |  8 +++++---
 3 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/rust/src/editing_crud.rs b/rust/src/editing_crud.rs
index 505bc6b4..405afcc0 100644
--- a/rust/src/editing_crud.rs
+++ b/rust/src/editing_crud.rs
@@ -33,7 +33,15 @@ pub trait EditorCrud {
 }
 impl EditorCrud for Editor {
     fn db_get(conn: &DbConn, editor_id: FatcatId) -> Result<EditorRow> {
-        let editor: EditorRow = editor::table.find(editor_id.to_uuid()).get_result(conn)?;
+        let editor: EditorRow = match editor::table.find(editor_id.to_uuid()).get_result(conn) {
+            Ok(ed) => ed,
+            Err(diesel::result::Error::NotFound) => {
+                return Err(
+                    FatcatError::NotFound("editor".to_string(), editor_id.to_string()).into(),
+                );
+            }
+            other => other?,
+        };
         Ok(editor)
     }
 
@@ -105,10 +113,21 @@ impl EditgroupCrud for Editgroup {
         conn: &DbConn,
         editgroup_id: FatcatId,
     ) -> Result<(EditgroupRow, Option<ChangelogRow>)> {
-        let (eg_row, cl_row): (EditgroupRow, Option<ChangelogRow>) = editgroup::table
+        let (eg_row, cl_row): (EditgroupRow, Option<ChangelogRow>) = match editgroup::table
             .left_outer_join(changelog::table)
             .filter(editgroup::id.eq(editgroup_id.to_uuid()))
-            .first(conn)?;
+            .first(conn)
+        {
+            Ok(eg) => eg,
+            Err(diesel::result::Error::NotFound) => {
+                return Err(FatcatError::NotFound(
+                    "editgroup".to_string(),
+                    editgroup_id.to_string(),
+                )
+                .into());
+            }
+            other => other?,
+        };
 
         ensure!(
             cl_row.is_some() == eg_row.is_accepted,
@@ -277,9 +296,20 @@ pub trait EditgroupAnnotationCrud {
 
 impl EditgroupAnnotationCrud for EditgroupAnnotation {
     fn db_get(conn: &DbConn, annotation_id: Uuid) -> Result<EditgroupAnnotationRow> {
-        let row: EditgroupAnnotationRow = editgroup_annotation::table
+        let row: EditgroupAnnotationRow = match editgroup_annotation::table
             .find(annotation_id)
-            .get_result(conn)?;
+            .get_result(conn)
+        {
+            Ok(ea) => ea,
+            Err(diesel::result::Error::NotFound) => {
+                return Err(FatcatError::NotFound(
+                    "editgroup_annotation".to_string(),
+                    annotation_id.to_string(),
+                )
+                .into());
+            }
+            other => other?,
+        };
         Ok(row)
     }
 
diff --git a/rust/src/endpoints.rs b/rust/src/endpoints.rs
index 2e467957..4271848a 100644
--- a/rust/src/endpoints.rs
+++ b/rust/src/endpoints.rs
@@ -34,7 +34,7 @@ macro_rules! generic_auth_err_responses {
     ($val:ident, $resp_type:ident) => {
         //use crate::errors::FatcatError::*;
         match $val {
-            NotFound(_, _) => $resp_type::NotFound($val.into()),
+            NotFound(_, _) | DatabaseRowNotFound => $resp_type::NotFound($val.into()),
             InvalidCredentials(_) | InsufficientPrivileges(_) => $resp_type::Forbidden($val.into()),
             DatabaseError(_) | InternalError(_) => {
                 error!("{}", $val);
@@ -50,7 +50,7 @@ macro_rules! generic_err_responses {
     ($val:ident, $resp_type:ident) => {
         //use crate::errors::FatcatError::*;
         match $val {
-            NotFound(_, _) => $resp_type::NotFound($val.into()),
+            NotFound(_, _) | DatabaseRowNotFound => $resp_type::NotFound($val.into()),
             DatabaseError(_) | InternalError(_) => {
                 error!("{}", $val);
                 capture_fail(&$val);
@@ -967,7 +967,9 @@ impl Api for Server {
                 CreateEditgroupResponse::SuccessfullyCreated(eg)
             }
             Err(fe) => match fe {
-                NotFound(_, _) => CreateEditgroupResponse::NotFound(fe.into()),
+                NotFound(_, _) | DatabaseRowNotFound => {
+                    CreateEditgroupResponse::NotFound(fe.into())
+                }
                 DatabaseError(_) | InternalError(_) => {
                     error!("{}", fe);
                     capture_fail(&fe);
@@ -1137,6 +1139,9 @@ impl Api for Server {
                 AuthOidcResponse::Found(result)
             }
             Err(fe) => match fe {
+                InvalidCredentials(_) | InsufficientPrivileges(_) => {
+                    AuthOidcResponse::Forbidden(fe.into())
+                }
                 DatabaseError(_) | InternalError(_) => {
                     error!("{}", fe);
                     capture_fail(&fe);
@@ -1182,6 +1187,9 @@ impl Api for Server {
                 message: "auth check successful!".to_string(),
             }),
             Err(fe) => match fe {
+                InvalidCredentials(_) | InsufficientPrivileges(_) => {
+                    AuthCheckResponse::Forbidden(fe.into())
+                }
                 DatabaseError(_) | InternalError(_) => {
                     error!("{}", fe);
                     capture_fail(&fe);
diff --git a/rust/src/errors.rs b/rust/src/errors.rs
index 08e79d51..cb53e6d1 100644
--- a/rust/src/errors.rs
+++ b/rust/src/errors.rs
@@ -85,6 +85,10 @@ pub enum FatcatError {
     // Diesel constraint that we think is a user error
     ConstraintViolation(String),
 
+    #[fail(display = "generic database 'not-found'")]
+    // This should generally get caught and handled
+    DatabaseRowNotFound,
+
     // TODO: can these hold context instead of Inner?
     #[fail(display = "unexpected database error: {}", _0)]
     // other Diesel, R2d2 errors which we don't think are user errors (eg, connection failure)
@@ -112,9 +116,7 @@ impl Into<models::ErrorResponse> for FatcatError {
 impl From<diesel::result::Error> for FatcatError {
     fn from(inner: diesel::result::Error) -> FatcatError {
         match inner {
-            diesel::result::Error::NotFound => {
-                FatcatError::NotFound("unknown".to_string(), "N/A".to_string())
-            }
+            diesel::result::Error::NotFound => FatcatError::DatabaseRowNotFound,
             diesel::result::Error::DatabaseError(_, _) => {
                 FatcatError::ConstraintViolation(inner.to_string())
             }
-- 
cgit v1.2.3