From a2184f56741cebd2763a147e899639f8eee8d31b Mon Sep 17 00:00:00 2001 From: Gursharan Singh <3442979+G8XSU@users.noreply.github.com> Date: Thu, 14 Sep 2023 14:25:19 -0700 Subject: [PATCH] Add NoSuchKeyError as possible errorcode Motivation: Such checks are best performed in backend than having client checking empty or null value. This is also in-line with more critical requirement for functioning of NotFound introduced by MonitorUpdatingPersister Design. Even though there are some popular KVStore's which do not throw such exception on keyNotFound such as AWS-DDB (arguably the most popular one), we can still figure out that key didn't exist if value was returned as null or by similar means in other stores. --- build.rs | 2 +- src/error.rs | 15 +++++++++++++++ src/types.rs | 17 ++++++++++++----- tests/tests.rs | 24 ++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/build.rs b/build.rs index 09e2db9..3ffff30 100644 --- a/build.rs +++ b/build.rs @@ -14,7 +14,7 @@ fn main() { #[cfg(feature = "genproto")] fn generate_protos() { download_file( - "https://raw.githubusercontent.com/lightningdevkit/vss-server/e1a88afd61f56d7e8e90a32036ca12389e36fe44/app/src/main/proto/vss.proto", + "https://raw.githubusercontent.com/lightningdevkit/vss-server/62e888e1bd3305d23b15da857edffaf527163048/app/src/main/proto/vss.proto", "src/proto/vss.proto", ).unwrap(); diff --git a/src/error.rs b/src/error.rs index d07aa77..07aa32d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,9 +10,20 @@ use std::fmt::{Display, Formatter}; /// information regarding each error code and corresponding use-cases. #[derive(Debug)] pub enum VssError { + /// Please refer to [`ErrorCode::NoSuchKeyException`]. + NoSuchKeyError(String), + + /// Please refer to [`ErrorCode::InvalidRequestException`]. InvalidRequestError(String), + + /// Please refer to [`ErrorCode::ConflictException`]. ConflictError(String), + + /// Please refer to [`ErrorCode::InternalServerException`]. InternalServerError(String), + + /// There is an unknown error, it could be a client-side bug, unrecognized error-code, network error + /// or something else. InternalError(String), } @@ -33,6 +44,9 @@ impl VssError { impl Display for VssError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { + VssError::NoSuchKeyError(message) => { + write!(f, "Requested key does not exist: {}", message) + } VssError::InvalidRequestError(message) => { write!(f, "Request sent to VSS Storage was invalid: {}", message) } @@ -54,6 +68,7 @@ impl Error for VssError {} impl From for VssError { fn from(error_response: ErrorResponse) -> Self { match error_response.error_code() { + ErrorCode::NoSuchKeyException => VssError::NoSuchKeyError(error_response.message), ErrorCode::InvalidRequestException => VssError::InvalidRequestError(error_response.message), ErrorCode::ConflictException => VssError::ConflictError(error_response.message), ErrorCode::InternalServerException => VssError::InternalServerError(error_response.message), diff --git a/src/types.rs b/src/types.rs index 252e3aa..0967782 100644 --- a/src/types.rs +++ b/src/types.rs @@ -10,7 +10,10 @@ pub struct GetObjectRequest { /// Authorization and billing can also be performed at the `store_id` level. #[prost(string, tag = "1")] pub store_id: ::prost::alloc::string::String, - /// `Key` for which the value is to be fetched. + /// The key of the value to be fetched. + /// + /// If the specified `key` does not exist, returns `ErrorCode.NO_SUCH_KEY_EXCEPTION` in the + /// the `ErrorResponse`. /// /// Consistency Guarantee: /// Get(read) operations against a `key` are consistent reads and will reflect all previous writes, @@ -287,17 +290,19 @@ pub struct KeyValue { pub enum ErrorCode { /// Default protobuf Enum value. Will not be used as `ErrorCode` by server. Unknown = 0, - /// CONFLICT_EXCEPTION is used when the request contains mismatched version (either key or global) + /// Used when the request contains mismatched version (either key or global) /// in `PutObjectRequest`. For more info refer `PutObjectRequest`. ConflictException = 1, - /// INVALID_REQUEST_EXCEPTION is used in the following cases: + /// Used in the following cases: /// - The request was missing a required argument. /// - The specified argument was invalid, incomplete or in the wrong format. /// - The request body of api cannot be deserialized into corresponding protobuf object. InvalidRequestException = 2, - /// An internal server error occurred, client is probably at no fault and can safely retry this - /// error with exponential backoff. + /// Used when an internal server error occurred, client is probably at no fault and can safely retry + /// this error with exponential backoff. InternalServerException = 3, + /// Used when the specified `key` in a `GetObjectRequest` does not exist. + NoSuchKeyException = 4, } impl ErrorCode { /// String value of the enum field names used in the ProtoBuf definition. @@ -310,6 +315,7 @@ impl ErrorCode { ErrorCode::ConflictException => "CONFLICT_EXCEPTION", ErrorCode::InvalidRequestException => "INVALID_REQUEST_EXCEPTION", ErrorCode::InternalServerException => "INTERNAL_SERVER_EXCEPTION", + ErrorCode::NoSuchKeyException => "NO_SUCH_KEY_EXCEPTION", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -319,6 +325,7 @@ impl ErrorCode { "CONFLICT_EXCEPTION" => Some(Self::ConflictException), "INVALID_REQUEST_EXCEPTION" => Some(Self::InvalidRequestException), "INTERNAL_SERVER_EXCEPTION" => Some(Self::InternalServerException), + "NO_SUCH_KEY_EXCEPTION" => Some(Self::NoSuchKeyException), _ => None, } } diff --git a/tests/tests.rs b/tests/tests.rs index 610d988..d5f6d62 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -149,6 +149,30 @@ mod tests { mock_server.expect(1).assert(); } + #[tokio::test] + async fn test_no_such_key_err_handling() { + let base_url = mockito::server_url(); + let vss_client = VssClient::new(&base_url); + + // NoSuchKeyError + let error_response = ErrorResponse { + error_code: ErrorCode::NoSuchKeyException.into(), + message: "NoSuchKeyException".to_string(), + }; + let mock_server = mockito::mock("POST", GET_OBJECT_ENDPOINT) + .with_status(409) + .with_body(&error_response.encode_to_vec()) + .create(); + + let get_result = vss_client + .get_object(&GetObjectRequest { store_id: "store".to_string(), key: "non_existent_key".to_string() }) + .await; + assert!(matches!(get_result.unwrap_err(), VssError::NoSuchKeyError { .. })); + + // Verify 1 request hit the server + mock_server.expect(1).assert(); + } + #[tokio::test] async fn test_invalid_request_err_handling() { let base_url = mockito::server_url();