feat(error): differentiate zome call error responses

jost-s · jost-s · commit 9e723295056d · 2025-03-12T12:38:28.000-06:00
diff --git a/src/error.rs b/src/error.rs
@@ -1,12 +1,11 @@
 //! hc-http-gw error types
 
+use crate::app_selection::AppSelectionError;
 use axum::http::StatusCode;
 use axum::response::IntoResponse;
 use axum::Json;
 use serde::{Deserialize, Serialize};
 
-use crate::app_selection::AppSelectionError;
-
 /// Core HTTP Gateway error type
 #[derive(thiserror::Error, Debug)]
 pub enum HcHttpGatewayError {
@@ -36,7 +35,7 @@ pub enum HcHttpGatewayError {
     AppSelectionError(#[from] AppSelectionError),
 }
 
-/// Type aliased Result
+/// Gateway result type.
 pub type HcHttpGatewayResult<T> = Result<T, HcHttpGatewayError>;
 
 /// Error format returned to the caller.
@@ -52,52 +51,38 @@ impl From<String> for ErrorResponse {
     }
 }
 
-impl From<&str> for ErrorResponse {
-    fn from(value: &str) -> Self {
-        Self {
-            error: value.to_owned(),
-        }
-    }
-}
-
-impl IntoResponse for HcHttpGatewayError {
-    fn into_response(self) -> axum::response::Response {
+impl HcHttpGatewayError {
+    /// Convert error into HTTP status code and error message.
+    pub fn into_status_code_and_body(self) -> (StatusCode, String) {
         match self {
-            HcHttpGatewayError::RequestMalformed(e) => (
-                StatusCode::BAD_REQUEST,
-                Json(ErrorResponse::from(format!("Request is malformed: {e}"))),
-            ),
-            HcHttpGatewayError::UnauthorizedFunction {
-                app_id,
-                zome_name,
-                fn_name,
-            } => (
-                StatusCode::FORBIDDEN,
-                Json(ErrorResponse::from(format!(
-                    "Function {fn_name} in zome {zome_name} in app {app_id} is not allowed"
-                ))),
-            ),
+            HcHttpGatewayError::RequestMalformed(_) => (StatusCode::BAD_REQUEST, self.to_string()),
+            HcHttpGatewayError::UnauthorizedFunction { .. } => {
+                (StatusCode::FORBIDDEN, self.to_string())
+            }
             HcHttpGatewayError::UpstreamUnavailable => (
                 StatusCode::BAD_GATEWAY,
-                Json(ErrorResponse::from("Could not connect to Holochain")),
-            ),
-            HcHttpGatewayError::AppSelectionError(AppSelectionError::NotInstalled) => (
-                StatusCode::NOT_FOUND,
-                Json(ErrorResponse::from(self.to_string())),
-            ),
-            HcHttpGatewayError::AppSelectionError(AppSelectionError::NotAllowed) => (
-                StatusCode::FORBIDDEN,
-                Json(ErrorResponse::from(self.to_string())),
-            ),
-            HcHttpGatewayError::AppSelectionError(AppSelectionError::MultipleMatching) => (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(ErrorResponse::from(self.to_string())),
+                "Could not connect to Holochain".to_string(),
             ),
+            HcHttpGatewayError::AppSelectionError(AppSelectionError::NotInstalled) => {
+                (StatusCode::NOT_FOUND, self.to_string())
+            }
+            HcHttpGatewayError::AppSelectionError(AppSelectionError::NotAllowed) => {
+                (StatusCode::FORBIDDEN, self.to_string())
+            }
+            HcHttpGatewayError::AppSelectionError(AppSelectionError::MultipleMatching) => {
+                (StatusCode::INTERNAL_SERVER_ERROR, self.to_string())
+            }
             _ => (
                 StatusCode::INTERNAL_SERVER_ERROR,
-                Json(ErrorResponse::from("Something went wrong")),
+                "Something went wrong".to_string(),
             ),
         }
-        .into_response()
+    }
+}
+
+impl IntoResponse for HcHttpGatewayError {
+    fn into_response(self) -> axum::response::Response {
+        let (status_code, body) = self.into_status_code_and_body();
+        (status_code, Json(ErrorResponse::from(body))).into_response()
     }
 }
diff --git a/src/holochain/app_conn_pool.rs b/src/holochain/app_conn_pool.rs
@@ -346,23 +346,32 @@ impl AppCall for AppConnPool {
         payload: ExternIO,
     ) -> BoxFuture<'static, HcHttpGatewayResult<ExternIO>> {
         let this = self.clone();
+        let app_id = installed_app_id.clone();
         Box::pin(async move {
             this.call(installed_app_id, |app_ws| {
+                let app_id = app_id.clone();
                 let cell_id = cell_id.clone();
                 let zome_name = zome_name.clone();
                 let fn_name = fn_name.clone();
                 let payload = payload.clone();
                 Box::pin(async move {
                     let result = app_ws
                         .call_zome(
-                            ZomeCallTarget::CellId(cell_id),
-                            zome_name.into(),
-                            fn_name.into(),
+                            ZomeCallTarget::CellId(cell_id.clone()),
+                            zome_name.clone().into(),
+                            fn_name.clone().into(),
                             payload,
                         )
                         .await;
                     if let Err(err) = &result {
-                        tracing::debug!(?err);
+                        tracing::debug!(
+                            ?err,
+                            ?app_id,
+                            ?cell_id,
+                            ?zome_name,
+                            ?fn_name,
+                            "Zome call error"
+                        );
                     }
                     let result = result?;
                     Ok(result)
diff --git a/src/routes/zome_call/tests.rs b/src/routes/zome_call/tests.rs
@@ -1,2 +1,5 @@
+// DnaHash::from_raw_32(vec![1; 32]).to_string()
+const DNA_HASH: &str = "uhC0kAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQF-z86-";
+
 mod responses;
 mod validations;
diff --git a/src/routes/zome_call/tests/responses.rs b/src/routes/zome_call/tests/responses.rs
@@ -1,52 +1,202 @@
+use super::DNA_HASH;
 use crate::config::{AllowedFns, Configuration};
 use crate::test::data::new_test_app_info;
 use crate::test::router::TestRouter;
 use crate::{MockAdminCall, MockAppCall};
-use holochain_client::ExternIO;
+use holochain_client::{ConductorApiError, ExternIO};
+use holochain_conductor_api::ExternalApiWireError;
 use holochain_types::prelude::DnaHash;
 use reqwest::StatusCode;
 use std::collections::HashMap;
 use std::net::{Ipv4Addr, SocketAddr};
 use std::sync::Arc;
 
-#[tokio::test(flavor = "multi_thread")]
-async fn happy_zome_call() {
-    let app_id = "tapp";
-    let dna_hash = DnaHash::from_raw_32(vec![1; 32]);
+const APP_ID: &str = "tapp";
 
+fn create_test_router(app_call: MockAppCall) -> TestRouter {
     let mut allowed_fns = HashMap::new();
-    allowed_fns.insert(app_id.into(), AllowedFns::All);
+    allowed_fns.insert(APP_ID.into(), AllowedFns::All);
     let config = Configuration::try_new(
         SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 8888),
         "1024",
-        app_id,
+        APP_ID,
         allowed_fns,
         "",
         "",
     )
     .unwrap();
 
     let mut admin_call = MockAdminCall::new();
-    let dna_hash2 = dna_hash.clone();
     admin_call.expect_list_apps().returning(move |_| {
-        let dna_hash = dna_hash2.clone();
         Box::pin(async move {
-            let app_info = new_test_app_info(app_id, dna_hash);
+            let app_info = new_test_app_info(APP_ID, DnaHash::from_raw_32(vec![1; 32]));
             Ok(vec![app_info])
         })
     });
     let admin_call = Arc::new(admin_call);
+    let app_call = Arc::new(app_call);
+    TestRouter::new_with_config_and_interfaces(config, admin_call, app_call)
+}
+
+#[tokio::test]
+async fn happy_zome_call() {
     let mut app_call = MockAppCall::new();
     app_call
         .expect_handle_zome_call()
         .returning(|_, _, _, _, _| {
             Box::pin(async move { Ok(ExternIO::encode("return_value").unwrap()) })
         });
-    let app_call = Arc::new(app_call);
-    let router = TestRouter::new_with_config_and_interfaces(config, admin_call, app_call);
+    let router = create_test_router(app_call);
     let (status_code, body) = router
-        .request(&format!("/{dna_hash}/{app_id}/coordinator/fn_name"))
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
         .await;
     assert_eq!(status_code, StatusCode::OK);
     assert_eq!(body, r#""return_value""#);
 }
+
+#[tokio::test]
+async fn app_not_found() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::AppNotFound,
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    // The app must have been found earlier when looking it up for the call,
+    // so this must have been an internal error of some kind.
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
+
+#[tokio::test]
+async fn cell_not_found() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::CellNotFound,
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
+
+#[tokio::test]
+async fn external_api_wire_error() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::ExternalApiWireError(
+                        ExternalApiWireError::ZomeCallUnauthorized("unauthorized".to_string()),
+                    ),
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
+
+#[tokio::test]
+async fn fresh_nonce_error() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::FreshNonceError("nonce_kaputt".into()),
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
+
+#[tokio::test]
+async fn io_error() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::IoError(std::io::Error::other("ssd not found")),
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
+
+#[tokio::test]
+async fn sign_zome_call_error() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::SignZomeCallError("unsigned".to_string()),
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
+
+#[tokio::test]
+async fn websocket_error() {
+    let mut app_call = MockAppCall::new();
+    app_call
+        .expect_handle_zome_call()
+        .returning(|_, _, _, _, _| {
+            Box::pin(async move {
+                Err(crate::HcHttpGatewayError::HolochainError(
+                    ConductorApiError::WebsocketError(
+                        // WebsocketError is not exposed.
+                        std::io::Error::other("websocket closed").into(),
+                    ),
+                ))
+            })
+        });
+    let router = create_test_router(app_call);
+    let (status_code, body) = router
+        .request(&format!("/{DNA_HASH}/{APP_ID}/coordinator/fn_name"))
+        .await;
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+    assert_eq!(body, r#"{"error":"Something went wrong"}"#);
+}
diff --git a/src/routes/zome_call/tests/validations.rs b/src/routes/zome_call/tests/validations.rs
@@ -1,3 +1,4 @@
+use super::DNA_HASH;
 use crate::test::router::TestRouter;
 use crate::test::test_tracing::initialize_testing_tracing_subscriber;
 use crate::{
@@ -9,9 +10,6 @@ use reqwest::StatusCode;
 use std::collections::HashMap;
 use std::net::{Ipv4Addr, SocketAddr};
 
-// DnaHash::from_raw_32(vec![1; 32]).to_string()
-const DNA_HASH: &str = "uhC0kAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQF-z86-";
-
 #[tokio::test]
 async fn valid_dna_hash_is_accepted() {
     initialize_testing_tracing_subscriber();
@@ -101,7 +99,7 @@ async fn unauthorized_function_name_is_rejected() {
     assert_eq!(
         body,
         format!(
-            r#"{{"error":"Function {fn_name} in zome zome_name in app coordinator is not allowed"}}"#
+            r#"{{"error":"Function {fn_name} in zome zome_name in app coordinator is not authorized"}}"#
         )
     );
 }
diff --git a/src/test/router.rs b/src/test/router.rs
