Consider removing feature-gating of backtraces in 2.0

[apollo-sturdy]

Now that backtraces API is stabilized since Rust 1.65.0 (2022-11-03), I would like to raise the idea of removing the feature-gating of backtraces. Backtraces are very helpful in debugging contract errors, but unfortunately the fact that they are currently feature-gated means that most packages that depend on cosmwasm-std do not support them and construct StdErrors without the backtrace field (one example here), which means that if you try to depend on such a package and enable the backtraces feature, compilation will fail since dependencies share features. If we remove the feature-gating then all packages depending on cosmwasm-std will require to add backtraces when constructing StdErrors, so this would be a breaking change. Something to consider for the next major version.

[webmaster128]

The backtraces add a lot of overhead at runtime which we don't want to have by default in Wasm, especially since error can be handled at runtime. They should be as lightweight as possible. In #1578 we'll remove a bunch of String allocations from errors for this reason.

which means that if you try to depend on such a package and enable the backtraces feature, compilation will fail

Yeah, I see. I recommend using the StdError constructors instead of the structs. E.g. cosmwasm_std::StdError::generic_err(msg) and friends. This has the same public API no matter which feature you have enabled.

But strictly speaking you have a point here. Enabling an additional feature should not break any public interface.

[apollo-sturdy]

@webmaster128 Ah I didn't know about the runtime overhead. I guess then maybe it makes sense to keep it feature gated. The other option would be to make the feature default and disable it when compiling for production, although I suppose then some people might forget (although they should really be using rust-optimizer which could do it for them).

Good tip on cosmwasm_std::StdError::generic_err(msg). Unfortunately I see almost no one using this :(
Just now again I found another example of the same problem in the pyth SDK: https://github.com/pyth-network/pyth-crosschain/blob/main/target_chains/cosmwasm/sdk/rust/src/error.rs#L79

[webmaster128]

Looking at https://doc.rust-lang.org/stable/src/std/backtrace.rs.html#276-299 it seems like just adding them in all cases would be a noop. @chipshort WDYT about making all fields

        #[cfg(feature = "backtraces")]
        backtrace: Backtrace,

non-feature-gated and fill them either with a backtrace or Backtrace::disabled()?

[chipshort]

So, the field would not be feature-gated, but if the feature is not set, we fill it with Backtrace::disabled() or if it is set, we fill with Backtrace::force_capture().
That will make the errors a few bytes bigger, but other than that it's indeed a noop. And it would make the feature non-breaking from that point on.
Let's do it 👍

[webmaster128]

we fill it with Backtrace::disabled() or if it is set, we fill with Backtrace::force_capture().

That's one option. The other way would be using Backtrace::capture() in all cases and drop the feature from this repo entirely. capture() does the distinction based on env variables. Maybe it would save some runtime code to to use Backtrace::disabled() when compiled to Wasm and Backtrace::capture() otherwise. But this also makes our feature needless which is great.

[chipshort]

Oh, that's a great idea. I didn't even think of that. Then you would never get them in wasm, but can enable them when using the env var when testing.

[webmaster128]

Unfortunately we run into dtolnay/thiserror#236 / dtolnay/thiserror#204 when we do this. Any idea to be able to use thiserror together with backtrace fields?

[mina86]

Please keep in mind that Backtrace is std so having the field always present may complicate no_std support.

[webmaster128]

Please keep in mind that Backtrace is std so having the field always present may complicate no_std support.

Good point. In this case I suggest some generic container field (Option or Box or something) with conditional contents and construction logic.