From 9302ffed95ce51fcbeaae6f19d9c599c490f2ea6 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 12 Apr 2022 21:15:30 -0700 Subject: [PATCH 1/3] report better error messages after Diesel queries --- nexus/src/db/error.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/nexus/src/db/error.rs b/nexus/src/db/error.rs index ddf703566a..b6d50aa0c9 100644 --- a/nexus/src/db/error.rs +++ b/nexus/src/db/error.rs @@ -183,10 +183,23 @@ where DieselError::DatabaseError(kind, info) => { PublicError::internal_error(&format_database_error(kind, &*info)) } - error => PublicError::internal_error(&format!( - "Unknown diesel error: {:?}", - error - )), + error => { + let context = match make_not_found_error() { + PublicError::ObjectNotFound { type_name, lookup_type } => { + format!( + "looking up {:?} {:?}", + type_name, lookup_type + ) + } + _ => { + format!("during unknown operation") + } + }; + PublicError::internal_error(&format!( + "Unknown diesel error {}: {:#}", + context, error + )) + } } } @@ -210,8 +223,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 )), } } From 12828afd315aa8454e5f1a22d74382b58717a193 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 12 Apr 2022 21:46:13 -0700 Subject: [PATCH 2/3] fix style --- nexus/src/db/error.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/nexus/src/db/error.rs b/nexus/src/db/error.rs index b6d50aa0c9..3811aa9dd3 100644 --- a/nexus/src/db/error.rs +++ b/nexus/src/db/error.rs @@ -186,10 +186,7 @@ where error => { let context = match make_not_found_error() { PublicError::ObjectNotFound { type_name, lookup_type } => { - format!( - "looking up {:?} {:?}", - type_name, lookup_type - ) + format!("looking up {:?} {:?}", type_name, lookup_type) } _ => { format!("during unknown operation") From bc738ba76faacd2afc1247a8dcf2cd5e99cc5b1e Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 13 Apr 2022 11:21:44 -0700 Subject: [PATCH 3/3] clean up implementation --- nexus/src/authz/api_resources.rs | 7 +++- nexus/src/authz/authz-macros/src/lib.rs | 10 +++--- nexus/src/db/error.rs | 43 +++++++++++-------------- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index df09dedac5..be6bb433fa 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -68,7 +68,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 AuthorizedResource diff --git a/nexus/src/authz/authz-macros/src/lib.rs b/nexus/src/authz/authz-macros/src/lib.rs index 0bfcac9dd8..0a93bd0257 100644 --- 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 index 3811aa9dd3..83327bcf6d 100644 --- 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,27 +172,21 @@ where /// Converts a Diesel error to an external error, handling "NotFound" using /// `make_not_found_error`. -fn public_error_from_diesel_lookup_helper( +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 => { - let context = match make_not_found_error() { - PublicError::ObjectNotFound { type_name, lookup_type } => { - format!("looking up {:?} {:?}", type_name, lookup_type) - } - _ => { - format!("during unknown operation") - } - }; + let context = + format!("accessing {:?} {:?}", resource_type, lookup_type); PublicError::internal_error(&format!( "Unknown diesel error {}: {:#}", context, error