search

tomusdrw / rust-web3

Ethereum JSON-RPC multi-transport client. Rust implementation of web3 library. ENS address: rust-web3.eth
MIT License
1.45k stars 471 forks source link

Fix panic when MetaMask returns error #549

Closed dvec closed 3 years ago

dvec commented 3 years ago

Fix #544

dvec commented 3 years ago

We can't run tests directly in browser, so I think the only solution is to mock window.ethereum, which may be a tricky task

dvec commented 3 years ago

Sounds like a solution, I'll try to commit it today

dvec commented 3 years ago

I'm wondering why there is a panic at all in case of invalid response while we have web3::error::Error::InvalidResponse. Especially considering that std::panic::catch_unwind is not available on wasm target (what also means that it's impossible to use #[should_panic] in tests). Can we refuse this behavior and return Err(web3::error::Error::InvalidResponse(...)) instead of panic?

tomusdrw commented 3 years ago

Can we refuse this behavior and return Err(web3::error::Error::InvalidResponse(...)) instead of panic?

Yeah, I just looked at the code and indeed we shouldn't be panicking and definitely the code is not unreachable as we can tell. If you want to bundle the fix in the same PR I'm fine with that.