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

Remove failure #151

Merged
merged 1 commit into from
Nov 16, 2019
Merged

Remove failure #151

merged 1 commit into from
Nov 16, 2019

Conversation

tony-iqlusion
Copy link
Member

@tony-iqlusion tony-iqlusion commented Nov 16, 2019

Since the time failure was written, std::error::Error has gained many of the features it was originally useful for, most notably Error::source for type-erased error chains/cascades (with downcast
support to recover the concrete type, if desired), and on nightly, backtrace support.

This commit attempts to remove failure in a way that largely preserves the existing error handling, replacing failure::Context with an internal abscissa_core::error::Context type which provides similar functionality, and more or less replaces the previous abscissa_core::error::Error type.

Additionally, it defines a BoxError alias for type-erased errors based on ideas from this rust-internals thread:

https://internals.rust-lang.org/t/proposal-add-std-boxerror/10953/1

It's bounded as Send + Sync + 'static to help in multithreaded/async contexts.

I'm not convinced this is good as a steady state, but rather could also be a stepping stone towards moving to some more widely supported library, as discussed in #144.

Since the time `failure` was written, `std::error::Error` has gained
many of the features it was originally useful for, most notably
`Error::source` for type erased error chains/cascades (with downcast
support to recover the concrete type, if desired), and on `nightly`,
backtrace support.

This commit attempts to remove `failure` in a way that largely preserves
the existing error handling, replacing `failure::Context` with an
internal `abscissa_core::error::Context` type which provides similar
functionality.

Additionally, it defines a `BoxError` alias for type-erased errors
based on ideas from this rust-internals thread:

https://internals.rust-lang.org/t/proposal-add-std-boxerror/10953/1

It's bounded as `Send + Sync + 'static` to help in multithreaded/async
contexts.

I'm not convinced this is good as a steady state, but rather could also
be a stepping stone towards moving to some more widely supported
library, as discussed in #144.
@tony-iqlusion tony-iqlusion added the dependency Issues about new, current, or alternative crate dependencies label Nov 16, 2019
@tony-iqlusion
Copy link
Member Author

tony-iqlusion commented Nov 16, 2019

There's definitely a lot more possible work that could be done on the error types (e.g. I'd like to eventually refactor abscissa_core::error::framework => abscissa_core::framework and drop the leading Framework* on the error types, using framework::Error instead.

However, this change seems like it's going to be somewhat high impact across my existing apps, so I think it'd be make sense to do this, ship a release and get everything updated to be failure-free, then pursue further changes, in order to make the process more incremental and lessen the impact.

To that end, this commit doesn't introduce any new error handling libraries (as discussed in #144).

@tarcieri
Copy link
Collaborator

Here are the current auto trait impls for abscissa_core::error::Context based on the current state of this PR:

Screen Shot 2019-11-16 at 9 35 58 AM

It's Send + Sync, which is good. I'm also wondering about UnwindSafe: would it be good to add a blanket impl for the case where the associated Kind is also UnwindSafe? That would also seem to depend on the unwind safety of BoxError, as well as Backtrace, which I'd have to investigate (so perhaps postpone that for another PR).

@@ -0,0 +1,92 @@
//! Error types
Copy link
Member Author

Choose a reason for hiding this comment

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

The CLI previously relied on failure::Error, which was kind of a hack. This file is in-line with what's in the current boilerplate for an error type.

@tony-iqlusion tony-iqlusion merged commit fb9d8ad into develop Nov 16, 2019
@tony-iqlusion tony-iqlusion deleted the remove-failure branch November 16, 2019 19:24
@tony-iqlusion tony-iqlusion mentioned this pull request Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependency Issues about new, current, or alternative crate dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants