wasm-bindgen
wasm-bindgen copied to clipboard
`UnwrapThrowExt` should propagate the error for `Result<T, E>`
Describe the Bug
I was debating between creating this issue as a feature request or as a bug, but I do believe it is a bug, as it is not what I would believe to be the intended behavior.
Currently, when calling unwrap_throw() or expect_throw() on an Err(E) variant, where E: Debug, only the message is printed, and not the debug value of E, as it does for the regular expect()/unwrap() counterparts. This makes the thrown error much less usable in diagnostics, especially since the unwrap_throw does not provide the std::panic:Location to hint as to what went wrong and where.
If this is, perhaps, a security concern, a feature flag might make this functionality available, and keep the old behavior when this flag is not set. Although, I am behind the idea of mimicking the original functions behavior when it comes to the thrown message.
Steps to Reproduce
- Try the following code:
use wasm_bindgen::prelude::*;
#[wasm_bindgen(start)]
fn start() {
let res: Result<(), String> = Err("This message should be displayed when thrown".to_string());
res.expect_throw("Throwing error...");
// or
res.unwrap_throw();
}
Expected Behavior
The unwrap_throw()/expect_throw() trait methods should mimic the behavior of the core/std library, displaying a debug view of the contained error value.
Actual Behavior
The expect_throw() method only displays the message in the thrown error, and neither unwrap_throw() nor expect_throw() display a std::panic::Location to trace the origin of the thrown error.
I think it'd be fine to update this to basically behave the same as the Rust standard library
@alexcrichton Based on the closed PR #2835, is this bug being labeled as wontfix, or is there a different implementation you'd be happy with? (I just spent an hour expanding macros to discover my real error was hidden by an unwrap_throw :-( )
For many/most uses the size savings of unwrap_throw aren't worth the extra debugging cost. In my case it was giving opaque errors, as well as triggering UB that led to red herring errors elsewhere.
See https://github.com/rustwasm/wasm-bindgen/pull/3899 for my benchmarks and proposed fix.