Repository: stellar/rs-soroban-env
Pull request #427 (2022-09-13 04:51:17)

Add invocation error debug event to call like try_call
### What
Add invocation error debug event to `Env::call` like `Env::try_call`.

### Why
A developer will choose call or try_call based on how they want to handle an error. In both cases diagnostics about the error are useful, and it is confusing that the diagnostic events are inconsistent. The fact the debug events are different implies something different has taken place inside the called contract, when the calls are the same.


Patch:
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index c55d6756..d1bebd65 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -1689,7 +1689,14 @@ impl VmCallerCheckedEnv for Host {
             self.charge_budget(CostType::CallArgsUnpack, hv.len() as u64)?;
             Ok(hv.iter().map(|a| a.to_raw()).collect())
         })?;
-        self.call_n(contract, func, args.as_slice())
+        let res = self.call_n(contract, func, args.as_slice());
+        if let Err(e) = &res {
+            let evt = DebugEvent::new()
+                .msg("contract call invocation resulted in error")
+                .arg::<RawVal>(e.status.into());
+            self.record_debug_event(evt)?;
+        }
+        res
     }
 
     // Notes on metering: covered by the components.
@@ -1702,13 +1709,7 @@ impl VmCallerCheckedEnv for Host {
     ) -> Result<RawVal, HostError> {
         match self.call(vmcaller, contract, func, args) {
             Ok(rv) => Ok(rv),
-            Err(e) => {
-                let evt = DebugEvent::new()
-                    .msg("try_call got error from callee contract")
-                    .arg::<RawVal>(e.status.clone().into());
-                self.record_debug_event(evt)?;
-                Ok(e.status.into())
-            }
+            Err(e) => Ok(e.status.into()),
         }
     }
 


Test patch:
diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs
index c80a85ec..fec31a13 100644
--- a/soroban-env-host/src/test/invocation.rs
+++ b/soroban-env-host/src/test/invocation.rs
@@ -2,8 +2,9 @@ use soroban_env_common::{xdr::ScVmErrorCode, RawVal};
 
 use crate::{
     budget::Budget,
+    events::{DebugArg, HostEvent},
     vm::Vm,
-    xdr::{Hash, ScHostObjErrorCode, ScVal, ScVec},
+    xdr::{Hash, ScHostObjErrorCode, ScStatusType, ScVal, ScVec},
     CheckedEnv, Host, HostError, Status, Symbol, Tag,
 };
 use soroban_test_wasms::{ADD_I32, INVOKE_CONTRACT, VEC};
@@ -70,10 +71,60 @@ fn invoke_cross_contract_with_err() -> Result<(), HostError> {
     let exp_st: Status = code.into();
     assert_eq!(sv.get_payload(), exp_st.to_raw().get_payload());
 
+    let events = host.get_events()?;
+    assert_eq!(events.0.len(), 3);
+    let last_event = events.0.last();
+    match last_event {
+        Some(HostEvent::Debug(de)) => {
+            assert_eq!(de.msg, Some("contract call invocation resulted in error"));
+            assert_eq!(de.args.len(), 1);
+            if let DebugArg::Val(rv) = de.args[0] {
+                let status: Status = rv.try_into()?;
+                assert_eq!(
+                    status,
+                    Status::from_type_and_code(
+                        ScStatusType::HostObjectError,
+                        ScHostObjErrorCode::VecIndexOutOfBound as u32,
+                    )
+                );
+            } else {
+                panic!("got: {:?}, want debug arg containing Val", de);
+            }
+        }
+        _ => {
+            panic!("got: {:?}, want debug event", last_event);
+        }
+    };
+
     // call
     let res = host.call(obj.to_object(), sym.into(), args.into());
     assert!(HostError::result_matches_err_status(res, code));
 
+    let events = host.get_events()?;
+    assert_eq!(events.0.len(), 6);
+    let last_event = events.0.last();
+    match last_event {
+        Some(HostEvent::Debug(de)) => {
+            assert_eq!(de.msg, Some("contract call invocation resulted in error"));
+            assert_eq!(de.args.len(), 1);
+            if let DebugArg::Val(rv) = de.args[0] {
+                let status: Status = rv.try_into()?;
+                assert_eq!(
+                    status,
+                    Status::from_type_and_code(
+                        ScStatusType::HostObjectError,
+                        ScHostObjErrorCode::VecIndexOutOfBound as u32,
+                    )
+                );
+            } else {
+                panic!("got: {:?}, want debug arg containing Val", de);
+            }
+        }
+        _ => {
+            panic!("got: {:?}, want debug event", last_event);
+        }
+    };
+
     Ok(())
 }
 
@@ -118,9 +169,66 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> {
     let sym = Symbol::from_str("add_with");
     let args = host.test_vec_obj::<i32>(&[i32::MAX, 1])?;
     let args = host.vec_push_back(args.val, id1_obj.into())?;
+
     // try call -- add will trap, and add_with will trap, but we will get a status
     let status = host.try_call(id0_obj.to_object(), sym.into(), args.clone().into())?;
-    let exp: Status = ScVmErrorCode::TrapUnreachable.into();
+    let code = ScVmErrorCode::TrapUnreachable;
+    let exp: Status = code.into();
     assert_eq!(status.get_payload(), exp.to_raw().get_payload());
+
+    let events = host.get_events()?;
+    assert_eq!(events.0.len(), 4);
+    let last_event = events.0.last();
+    match last_event {
+        Some(HostEvent::Debug(de)) => {
+            assert_eq!(de.msg, Some("contract call invocation resulted in error"));
+            assert_eq!(de.args.len(), 1);
+            if let DebugArg::Val(rv) = de.args[0] {
+                let status: Status = rv.try_into()?;
+                assert_eq!(
+                    status,
+                    Status::from_type_and_code(
+                        ScStatusType::VmError,
+                        ScVmErrorCode::TrapUnreachable as u32,
+                    )
+                );
+            } else {
+                panic!("got: {:?}, want debug arg containing Val", de);
+            }
+        }
+        _ => {
+            panic!("got: {:?}, want debug event", last_event);
+        }
+    };
+
+    // call
+    let res = host.call(id0_obj.to_object(), sym.into(), args.clone().into());
+    assert!(HostError::result_matches_err_status(res, code));
+
+    let events = host.get_events()?;
+    assert_eq!(events.0.len(), 8);
+    let last_event = events.0.last();
+    match last_event {
+        Some(HostEvent::Debug(de)) => {
+            assert_eq!(de.msg, Some("contract call invocation resulted in error"));
+            assert_eq!(de.args.len(), 1);
+            if let DebugArg::Val(rv) = de.args[0] {
+                let status: Status = rv.try_into()?;
+                assert_eq!(
+                    status,
+                    Status::from_type_and_code(
+                        ScStatusType::VmError,
+                        ScVmErrorCode::TrapUnreachable as u32,
+                    )
+                );
+            } else {
+                panic!("got: {:?}, want debug arg containing Val", de);
+            }
+        }
+        _ => {
+            panic!("got: {:?}, want debug event", last_event);
+        }
+    };
+
     Ok(())
 }