Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add context to multitest execution errors #597

Merged
merged 10 commits into from
Jan 4, 2022
25 changes: 25 additions & 0 deletions packages/multi-test/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2584,6 +2584,31 @@ mod test {
mod errors {
use super::*;

#[test]
fn simple_instantiation() {
let owner = Addr::unchecked("owner");
let mut app = App::default();

// set up contract
let code_id = app.store_code(error::contract(false));
let msg = EmptyMsg {};
let err = app
.instantiate_contract(code_id, owner, &msg, &[], "error", None)
.unwrap_err();

// we should be able to retrieve the original error by downcasting
let source: &StdError = err.downcast_ref().unwrap();
if let StdError::GenericErr { msg } = source {
assert_eq!(msg, "Init failed");
} else {
panic!("wrong StdError variant");
}

// we're expecting exactly 3 nested error types
// (the original error, initiate msg context, WasmMsg context)
assert_eq!(err.chain().count(), 3);
}

#[test]
fn simple_call() {
let owner = Addr::unchecked("owner");
Expand Down
26 changes: 18 additions & 8 deletions packages/multi-test/src/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,13 +319,13 @@ impl<T1, T2, T3, E1, E2, E3, C, T4, E4, E5, T6, E6> Contract<C>
for ContractWrapper<T1, T2, T3, E1, E2, E3, C, T4, E4, E5, T6, E6>
where
T1: DeserializeOwned + Debug + Clone,
T2: DeserializeOwned,
T3: DeserializeOwned,
T2: DeserializeOwned + Debug + Clone,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would remove Clone from T1, T2, T3

T3: DeserializeOwned + Debug + Clone,
T4: DeserializeOwned,
T6: DeserializeOwned,
E1: Display + Debug + Send + Sync + Error + 'static,
E2: Display + Debug + Send + Sync + 'static,
E3: Display + Debug + Send + Sync + 'static,
E2: Display + Debug + Send + Sync + Error + 'static,
E3: Display + Debug + Send + Sync + Error + 'static,
E4: Display + Debug + Send + Sync + 'static,
E5: Display + Debug + Send + Sync + 'static,
E6: Display + Debug + Send + Sync + 'static,
Expand Down Expand Up @@ -354,13 +354,23 @@ where
info: MessageInfo,
msg: Vec<u8>,
) -> AnyResult<Response<C>> {
let msg = from_slice(&msg)?;
(self.instantiate_fn)(deps, env, info, msg).map_err(|err| anyhow!(err))
let msg: T2 = from_slice(&msg)?;
(self.instantiate_fn)(deps, env, info, msg.clone())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, we could remove the Clone bound if you did the following, right?

let ctx = format!("Contract returned an error on instantiate msg:\n{:?}", msg);
(self.instantiate_fn)(deps, env, info, msg).map_err(anyhow::Error::from).context(ctx)

Nothing really important, just like reducing required bounds and removing clones when possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, yeah, that looks much better. Will do 👍

.map_err(anyhow::Error::from)
.context(format!(
"Contract returned an error on instantiate msg:\n{:?}",
msg,
))
}

fn query(&self, deps: Deps, env: Env, msg: Vec<u8>) -> AnyResult<Binary> {
let msg = from_slice(&msg)?;
(self.query_fn)(deps, env, msg).map_err(|err| anyhow!(err))
let msg: T3 = from_slice(&msg)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks to add these two and the simple test. Looks good to me

(self.query_fn)(deps, env, msg.clone())
.map_err(anyhow::Error::from)
.context(format!(
"Contract returned an error on query msg:\n{:?}",
msg,
))
}

// this returns an error if the contract doesn't implement sudo
Expand Down