report better error messages after Diesel queries (#907)

oxidecomputer · Apr 13, 2022 · 71a5326 · 71a5326
1 parent f380f83
commit 71a5326
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 27 deletions.
diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs
@@ -69,7 +69,12 @@ pub trait ApiResource: Clone + Send + Sync + 'static {
 pub trait ApiResourceError {
     /// Returns an error as though this resource were not found, suitable for
     /// use when an actor should not be able to see that this resource exists
-    fn not_found(&self) -> Error;
+    fn not_found(&self) -> Error {
+        self.lookup_type().clone().into_not_found(self.resource_type())
+    }
+
+    fn resource_type(&self) -> ResourceType;
+    fn lookup_type(&self) -> &LookupType;
 }
 
 impl<T: ApiResource + ApiResourceError + oso::PolarClass> AuthorizedResource

diff --git a/nexus/src/authz/authz-macros/src/lib.rs b/nexus/src/authz/authz-macros/src/lib.rs
@@ -344,10 +344,12 @@ fn do_authz_resource(
         }
 
         impl ApiResourceError for #resource_name {
-            fn not_found(&self) -> Error {
-                self.lookup_type
-                    .clone()
-                    .into_not_found(ResourceType::#resource_name)
+            fn resource_type(&self) -> ResourceType {
+                ResourceType::#resource_name
+            }
+
+            fn lookup_type(&self) -> &LookupType {
+                &self.lookup_type
             }
         }
     })

diff --git a/nexus/src/db/error.rs b/nexus/src/db/error.rs
@@ -121,17 +121,18 @@ pub fn public_error_from_diesel_pool(
 ) -> PublicError {
     public_error_from_diesel_pool_helper(error, |error| match handler {
         ErrorHandler::NotFoundByResource(resource) => {
-            public_error_from_diesel_lookup_helper(error, || {
-                resource.not_found()
-            })
+            public_error_from_diesel_lookup_helper(
+                error,
+                resource.resource_type(),
+                resource.lookup_type(),
+            )
         }
         ErrorHandler::NotFoundByLookup(resource_type, lookup_type) => {
-            public_error_from_diesel_lookup_helper(error, || {
-                PublicError::ObjectNotFound {
-                    type_name: resource_type,
-                    lookup_type,
-                }
-            })
+            public_error_from_diesel_lookup_helper(
+                error,
+                resource_type,
+                &lookup_type,
+            )
         }
         ErrorHandler::Conflict(resource_type, object_name) => {
             public_error_from_diesel_create(error, resource_type, object_name)
@@ -171,22 +172,26 @@ where
 
 /// Converts a Diesel error to an external error, handling "NotFound" using
 /// `make_not_found_error`.
-fn public_error_from_diesel_lookup_helper<F>(
+fn public_error_from_diesel_lookup_helper(
     error: DieselError,
-    make_not_found_error: F,
-) -> PublicError
-where
-    F: FnOnce() -> PublicError,
-{
+    resource_type: ResourceType,
+    lookup_type: &LookupType,
+) -> PublicError {
     match error {
-        DieselError::NotFound => make_not_found_error(),
+        DieselError::NotFound => {
+            lookup_type.clone().into_not_found(resource_type)
+        }
         DieselError::DatabaseError(kind, info) => {
             PublicError::internal_error(&format_database_error(kind, &*info))
         }
-        error => PublicError::internal_error(&format!(
-            "Unknown diesel error: {:?}",
-            error
-        )),
+        error => {
+            let context =
+                format!("accessing {:?} {:?}", resource_type, lookup_type);
+            PublicError::internal_error(&format!(
+                "Unknown diesel error {}: {:#}",
+                context, error
+            ))
+        }
     }
 }
 
@@ -210,8 +215,8 @@ fn public_error_from_diesel_create(
             )),
         },
         _ => PublicError::internal_error(&format!(
-            "Unknown diesel error: {:?}",
-            error
+            "Unknown diesel error creating {:?} called {:?}: {:#}",
+            resource_type, object_name, error
         )),
     }
 }