From b23fd20a99e001ed9063d9eb083a7e5ccd23a083 Mon Sep 17 00:00:00 2001
From: Enrico Marconi <enrico.marconi@hotmail.it>
Date: Mon, 27 Nov 2023 16:46:04 +0100
Subject: [PATCH] Better (?) error handling

---
 bindings/grpc/src/services/credential.rs      | 26 +++++++++++++++----
 .../tests/api/credential_revocation_check.rs  | 16 +++++++++---
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/bindings/grpc/src/services/credential.rs b/bindings/grpc/src/services/credential.rs
index 3c71041238..70a85b2d57 100644
--- a/bindings/grpc/src/services/credential.rs
+++ b/bindings/grpc/src/services/credential.rs
@@ -3,7 +3,7 @@ use credential_verification::{
   RevocationCheckRequest, RevocationCheckResponse, RevocationStatus,
 };
 use identity_iota::{
-  credential::{JwtCredentialValidatorUtils, JwtValidationError, RevocationBitmapStatus, self},
+  credential::{self, JwtCredentialValidatorUtils, JwtValidationError, RevocationBitmapStatus},
   prelude::{IotaDocument, Resolver},
 };
 use iota_sdk::client::Client;
@@ -54,11 +54,27 @@ pub enum RevocationCheckError {
   InvalidRevocationUrl(String),
   #[error("Properties isn't a valid JSON object")]
   MalformedPropertiesObject,
+  #[error("Invalid credential status")]
+  InvalidCredentialStatus(#[source] credential::Error),
+  #[error("Issuer's DID resolution error")]
+  ResolutionError(#[source] identity_iota::resolver::Error),
+  #[error("Revocation map not found")]
+  RevocationMapNotFound(#[source] credential::JwtValidationError),
 }
 
 impl From<RevocationCheckError> for tonic::Status {
   fn from(e: RevocationCheckError) -> Self {
-    Self::from_error(Box::new(e))
+    let message = e.to_string();
+    let code = match &e {
+      RevocationCheckError::InvalidCredentialStatus(_)
+        | RevocationCheckError::MalformedPropertiesObject
+        | RevocationCheckError::UnknownRevocationType(_)
+        | RevocationCheckError::InvalidRevocationUrl(_) => tonic::Code::InvalidArgument,
+      RevocationCheckError::ResolutionError(_) => tonic::Code::Internal,
+      RevocationCheckError::RevocationMapNotFound(_) => tonic::Code::NotFound,
+    };
+
+    tonic::Status::new(code, message)
   }
 }
 
@@ -83,14 +99,14 @@ impl CredentialRevocation for CredentialVerifier {
   ) -> Result<Response<RevocationCheckResponse>, tonic::Status> {
     let credential_revocation_status = {
       let revocation_status = credential::Status::try_from(req.into_inner())?;
-      RevocationBitmapStatus::try_from(revocation_status).map_err(|e| tonic::Status::from_error(Box::new(e)))?
+      RevocationBitmapStatus::try_from(revocation_status).map_err(RevocationCheckError::InvalidCredentialStatus)?
     };
     let issuer_did = credential_revocation_status.id().unwrap(); // Safety: already parsed as a valid URL
     let issuer_doc = self
       .resolver
       .resolve(issuer_did.did())
       .await
-      .map_err(|e| tonic::Status::from_error(Box::new(e)))?;
+      .map_err(RevocationCheckError::ResolutionError)?;
 
     if let Err(e) =
       JwtCredentialValidatorUtils::check_revocation_bitmap_status(&issuer_doc, credential_revocation_status)
@@ -99,7 +115,7 @@ impl CredentialRevocation for CredentialVerifier {
         JwtValidationError::Revoked => Ok(Response::new(RevocationCheckResponse {
           status: RevocationStatus::Revoked.into(),
         })),
-        _ => Err(tonic::Status::from_error(Box::new(e))),
+        _ => Err(RevocationCheckError::RevocationMapNotFound(e).into()),
       }
     } else {
       Ok(Response::new(RevocationCheckResponse {
diff --git a/bindings/grpc/tests/api/credential_revocation_check.rs b/bindings/grpc/tests/api/credential_revocation_check.rs
index 312b71f6a5..777233f06c 100644
--- a/bindings/grpc/tests/api/credential_revocation_check.rs
+++ b/bindings/grpc/tests/api/credential_revocation_check.rs
@@ -1,6 +1,6 @@
 use credentials::{credential_revocation_client::CredentialRevocationClient, RevocationStatus};
 use identity_iota::{
-  credential::{self, RevocationBitmap, RevocationBitmapStatus, StatusCheck},
+  credential::{self, RevocationBitmap, RevocationBitmapStatus},
   did::DID,
 };
 
@@ -14,7 +14,7 @@ mod credentials {
 }
 
 #[tokio::test]
-async fn checking_status_of_valid_credential_works() -> anyhow::Result<()> {
+async fn checking_status_of_credential_works() -> anyhow::Result<()> {
   let server = TestServer::new().await;
   let client = server.client();
   let mut issuer = Entity::new();
@@ -52,9 +52,19 @@ async fn checking_status_of_valid_credential_works() -> anyhow::Result<()> {
       .collect(),
   };
   dbg!(&req);
-  let res = grpc_client.check(tonic::Request::new(req)).await?.into_inner();
+  let res = grpc_client.check(tonic::Request::new(req.clone())).await?.into_inner();
 
   assert_eq!(res.status(), RevocationStatus::Valid);
 
+  // Revoke credential
+  issuer
+    .update_document(&client, |mut doc| {
+      doc.revoke_credentials("my-revocation-service", &[3]).ok().map(|_| doc)
+    })
+    .await?;
+
+  let res = grpc_client.check(tonic::Request::new(req)).await?.into_inner();
+  assert_eq!(res.status(), RevocationStatus::Revoked);
+
   Ok(())
 }