crates: Replacements for failure?

[tarcieri]

std::error::Error now contains a trait as expressive as the Fail trait from failure, including downcast support and downcastable Error::source as a replacement for Error::cause (now with a 'static lifetime bound).

Additionally, there are a number of custom derive crates for std::error::Error now available:

• err-derive
• snafu
• thiserror

Given that, it seems like we can eliminate failure as a dependency, potentially replacing failure_derive with one of the crates above.

Serializable Errors

A bit of a sidebar, but one of the things I'd like to eventually accomplish is serde support for the framework's Error type, making it Serialize-able (and ideally Deserialize-able as well), with the intent of serializing them as JSON for reporting to an exception reporting service.

The backtrace crate offers serde support as an optional feature, however accessing this functionality was previously difficult as Abscissa's Error type uses failure::Context to capture and store backtraces.
It seems like Abscissa could benefit from providing a similar Context type as a replacement for the one failure provided. Ideally this type could simply derive(Serialize, Deserialize), permitting straightforward serialization of errors as well as their backtraces (and ideally, the entire error chain).

Unfortunately the story around all of this is complicated by adding a Backtrace type to std, presently only available on nightly. This Backtrace type has deliberately minimal functionality.

I'd consider supporting Backtrace on stable a requirement, and ideally preserving the serde serialization support. This seems possible with snafu but needs more investigation.

Crates like err-derive and thiserror have the advantage of being pure proc macros with no additional API dependencies, so if nothing else they stay nicely out of the way and orthogonal to this problem. I imagine if we don't go with snafu, we could pick one of these to be the out-of-the-box custom derive for errors, but with the only framework-level association being inclusion in the default application template (allowing downstream users of the users to potentially remove it or swap it out if they so desire)

[hdevalence]

I think that this issue mixes two distinct questions, assuming the goal is to replace all uses of the Fail trait with the Error trait:

1. What crate, if any, should be used to derive Abscissa's error types;
2. What type (implies: from which crate) Abscissa should use as its catch-all type for working with errors within an Abscissa application.

Currently, Abscissa uses failure for (1) and failure::Error for (2). But these are actually different functional roles and should be considered differently. thiserror and err-derive are only useful for (1) and not (2), so choosing one or the other still leaves the choice for (2) open.

The answer to question (1) seems relatively less important to Abscissa users, because any answer to question (1) is an implementation detail for Abscissa. The answer to question (2) seems relatively more important, because any answer shows up in the public API.

Two possible answers to question (2) are:

1. type BoxedStdError = Box<dyn std::error::Error + Send + Sync + 'static>;
2. anyhow::Error, like a Box<dyn Error + Send + Sync + 'static> but with additional ergonomics.

As an Abscissa user, I would strongly prefer having all generic errors be bounded with Send + Sync + 'static to make using them with multithreaded async code easier. Using a BoxedStdError type alias probably prevents adding serialization support.

[tarcieri]

I'd be fine with defining a framework level boxed error type. See the "Proposal: add std::error::BoxError" rust-internals thread which explores various tradeoffs regarding how it should be defined.

The main problem with using that as "the" Error type, at least for now, is it can't carry a Backtrace.

anyhow solves this by using nightly's Backtrace type. Unfortunately that doesn't help stable users, or support backtrace serialization AFAIK. Longer term it seems nice though.

[hawkw]

The backtrace crate offers serde support as an optional feature, however accessing this functionality was previously difficult as Abscissa's Error type uses failure::Context to capture and store backtraces.
It seems like Abscissa could benefit from providing a similar Context type as a replacement for the one failure provided. Ideally this type could simply derive(Serialize, Deserialize), permitting straightforward serialization of errors as well as their backtraces (and ideally, the entire error chain).

FWIW, I just thought I'd drop in and add that I'm currently experimenting with a tracing-error crate (not yet published) that adds support for capturing the tracing span context in which an error was constructed, and including it with the error. This forms something not unlike a backtrace, but consisting of user-provided semantic contexts rather than stack frames (a stack trace could be captured as well). If this is something you're interested in, especially in light of #149, I'd be happy to let you know when this experiment is ready to start trying out.

[tarcieri]

@hawkw very cool. It'd be quite interesting if it were possible to trace things like asynchronous events which originate in an RPC request but the fire off some sort of background job (kind of like an in-process equivalent of the similar functionality in Dapper/Zipkin/OpenTracing).

a stack trace could be captured as well

I'd almost certainly want that

[tarcieri]

One quick note on backtrace capture in general: it seems like the best option there (and the direction the backtrace crate is heading) is to use gimli (as opposed to libbacktrace). The linked thread notes a history of security issues in libbacktrace, whereas gimli is written in Rust and should therefore be safer.

Unfortunately that pulls in a large number of dependencies today, so I'm wary to enable gimli by default until it lands in std and therefore those dependencies are linked as part of the Rust distribution itself and come at zero cost to Abscissa (which could then ideally leverage the currently nightly-only backtrace method defined on std::error::Error).

It would be easy enough to add an off-by-default Cargo feature to enable them, however, and a commented-out line in the application boilerplate template to optionally enable it (in which case I'd also turn it on in all of my personal apps).

[hawkw]

@hawkw very cool. It'd be quite interesting if it were possible to trace things like asynchronous events which originate in an RPC request but the fire off some sort of background job (kind of like an in-process equivalent of the similar functionality in Dapper/Zipkin/OpenTracing).

Yup, that's more or less the exact goal I had in mind! :)

One quick note on backtrace capture in general: it seems like the best option there (and the direction the backtrace crate is heading) is to use gimli (as opposed to libbacktrace). The linked thread notes a history of security issues in libbacktrace, whereas gimli is written in Rust and should therefore be safer.

Unfortunately that pulls in a large number of dependencies today, so I'm wary to enable gimli by default until it lands in std and therefore those dependencies are linked as part of the Rust distribution itself and come at zero cost to Abscissa (which could then ideally leverage the currently nightly-only backtrace method defined on std::error::Error).

When I get around to adding stack backtraces to a tracing-error crate, I'll definitely keep this in mind; until gimli lands in std, I think it will end up having separate feature flags for libbacktrace and gimli backtrace capture.

[tarcieri]

I'd be fine with defining a framework level boxed error type.

I added an abscissa_core::error::BoxError type alias in #151, bounded as Send + Sync + 'static:

https://github.com/iqlusioninc/abscissa/pull/151/files#diff-11635f54908685fd908957a138677245R14

[tarcieri]

Here's a comprehensive analysis of various popular Rust error handling libraries, what features they provide, and how they relate to each other:

https://blog.yoshuawuyts.com/error-handling-survey/

[tarcieri]

Now that v0.5.0 is out and I'm updating a lot of apps which previously used failure_derive, I've been using thiserror in pretty much all cases.

I think it'd be worth including in the framework boilerplate (but not necessarily as a framework-level dependency).

This could be done in a v0.5.1 release of the abscissa crate without any changes to abscissa_core.