Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Separate backtrace and std features? #127

Open
daboross opened this issue Jan 10, 2018 · 14 comments
Open

Separate backtrace and std features? #127

daboross opened this issue Jan 10, 2018 · 14 comments

Comments

@daboross
Copy link

daboross commented Jan 10, 2018

In Cargo.toml, the std feature depends on backtrace. Is there a technical reason this is required, or would it be possible to implement Fail for StdError without depending on backtrace?

We have a use case in dotenv for using failure crate with std error types as #[cause]s, but it would be ideal if we didn't pull in all of the backtrace crate.

Would it be possible to either:

  • add a new feature for impl<E: StdError + Send + Sync + 'static> Fail for E {} separate from backtrace and std
  • or have the std feature not depend on backtrace?

I'd be willing to submit a PR doing this if it's something you'd want to add, but there might be some other thing blocking it that I'm not aware of?

@withoutboats
Copy link
Contributor

You could instead make both std and backtrace default features, I believe.

@daboross
Copy link
Author

OK! Thank you for the quick response here.

Do you think it would be a breaking change to require the backtrace feature for backtrace functionality? Currently someone can do default-features = false, features = ["std"], and get backtrace functionality. That would no longer be the case.

In either case, I've submitted https://github.com/withoutboats/failure/pull/128 if that's something you'd want.

@radupopescu
Copy link

Hi!
I also discovered that the failure::Error type is only available with the std feature. I was trying to disable the dependency on the "backtrace" crate, in order to compile my program for the x86_64-unknown-linux-musl target.

I would be great having the ability to disable the backtrace depedency without losing failure::Error.

@withoutboats
Copy link
Contributor

I'm consider going a step further and making backtrace also not a default feature.

Its basically impossible not to use backtrace unless all of the libraries you use also turn it off. While in a no-std context this makes sense, for backtrace libraries are unlikely to turn it off because it doesn't impact them.

This would mean anyone who wants to use backtraces needs to turn the backtrace feature on in their Cargo.toml.

@radupopescu
Copy link

Hi there!

So if I understand correctly, you say that the backtrace becomes a separate feature, turned off by default. Sounds good!

This would mean that by default, the backtrace feature will not "bubble up" from the transitive closure of packages we depend on, unless one of them explicitly enables the feature in its own Cargo.toml?

Anyway, thanks for making the library! I haven't had issues moving from error-chain to failure, and I definitely prefer defining my own error types + deriving Fail + failure::Error to putting everything in a big macro, as was the case with error-chain. I also appreciate the freedom to mix functions that Result<T, E> where E: Fail with Result<T, Error>.

@daboross
Copy link
Author

daboross commented Jan 28, 2018

@radupopescu from my understanding of Cargo features, nothing will have backtraces until a package depends on failure = { version = "0.2", features = ["backtrace"] }. As long as one package depends on it with that feature though, backtrace support will be enabled everywhere.

@withoutboats
Copy link
Contributor

Closed by #144.

@daboross
Copy link
Author

daboross commented Aug 26, 2018

Could this be re-opened per 3d5b8f9?

This is still important to me and I'd really like it not to fall between the cracks. I love what failure does for error handling, but this is still blocking effective usage of failure without backtraces.

I'm 100% OK with backtraces being default as long as it is possible to disable them without also disabling std support.

@daboross
Copy link
Author

daboross commented Sep 3, 2018

@mitsuhiko (or current maintainer) any possibility this could be opened? I don't have the ability to re-open it.

I can make a new issue if necessary. I'd just really like the ability to use std without backtrace, and that ability was removed in the main branch as of 3d5b8f9. I can understand not wanting breaking changes, but it'd be nice to keep this issue open until those breaking changes are made so it's still on the radar?

@lucab
Copy link

lucab commented Sep 19, 2018

Gentle ping to @mitsuhiko and @rust-lang-nursery/failure. I'm also again in need of this and it would be better if re-opened.

@mitsuhiko mitsuhiko reopened this Sep 19, 2018
@mitsuhiko
Copy link
Contributor

If we can find a way to make this backwards compatible I'm accepting PRs :)

@lucab
Copy link

lucab commented Sep 19, 2018

@mitsuhiko I am not sure there is a way to do that, thus this was mostly so that it doesn't get forgotten for next breaking release.

Speaking of that, given that rust-lang/rust#53533 landed in nightly already, perhaps 0.2 work (in no-backtrace mode, for now) can already start somewhere?

@daboross
Copy link
Author

daboross commented Sep 19, 2018

Agreeing with @lucab here that doing this right would be a breaking change.

We could implement this without a breaking change, but the only way I can think to do it is to add an extra std2 feature identical to std but without depending on backtrace. This wouldn't change the defaults, but if someone depended on failure = { version = "0.1", default-features = false, features = ["std2"] } they could get std features without backtrace.

If 0.2 is anywhere close, adding std2 with plans to remove it at 0.2 probably wouldn't be worth it. Especially because it would mean asking all the libraries who depend on failure but don't use the result to switch to std2, release a new version, then switch back when failure 0.2 is released.

Edit: I've opened #257.

@benbrittain
Copy link

I too would appreciate if backtrace was optional.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants