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(-)
(limited to 'rust/src')
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