http-rs / http-types

Common types for HTTP operations
https://docs.rs/http-types
Apache License 2.0
200 stars 83 forks source link

Proposal: Switch http-types::error::Error.error from anyhow to eyre #214

Open jbr opened 4 years ago

jbr commented 4 years ago

https://lib.rs/crates/eyre is a fork of anyhow that uses the https://lib.rs/crates/backtrace instead of std::backtrace::Backtrace. the backtrace crate currently provides a much better interface than std::backtrace::Backtrace for programmatic interaction (iterating over frames, for example, allowing for backtrace filtering)

This is important for a web framework because the framework can provide a highly usable in-browser interface to understand errors. Here's an example from the phoenix framework: image

In development mode, they render a relevant snippet of code and a backtrace annotated with filesystem locations. IIRC, clicking any of those stack frames jumps to that line, syntax highlighted, in browser. This is obviously way beyond what http-types, rust, and tide can currently offer, but switching to eyre and the backtrace crate would allow us to take further steps in that direction.

jbr commented 4 years ago

cc @yoshuawuyts @Fishrock123 because there's no way to request review on an issue

yoshuawuyts commented 4 years ago

Not opposed to this in principle; I'm curious how a PR would turn out. The Elixir page you shared looks fantastic and enabling that would be amazing.

One gotcha tho is that (I believe) we recently introduced an explicit "from anyhow" conversion which we cannot remove without a major version bump. So at least for a while we would need to keep both dependencies in tree.

jbr commented 4 years ago

Yeah, I do think this would have to be a major release change. Unless eyre provides a From for eyre::Error, this would necessarily be a breaking change. Without specialization I don't think we can support both anyhow and eyre simultaneously (not sure about that, but would be pleasantly surprised if it was possible)

jbr commented 4 years ago

also see #213 for a screenshot from a tide app that's a step in the direction of the phoenix error page

jbr commented 4 years ago

On further investigation, eyre might not be the solution either due to yaahc/eyre#32. We might just want to capture our own backtrace::Backtrace. I'll take a look at that, shouldn't be too bad

Fishrock123 commented 4 years ago

From<anyhow::Error> for eyre::Error

That seems reasonable to request/PR to eyre?

jbr commented 4 years ago

@Fishrock123 there's a closed issue about that https://github.com/yaahc/eyre/issues/31 and we can't add our own From<anyhow::Error> for http_types::Error because of our generic From<E: Into<eyre::Error>> for http_types::Error (I believe we'd get one of those compiler errors about how anyhow could could implement Into<eyre::Error> for anyhow::Error or something like that)

jbr commented 4 years ago

We weren't anyhow compatible prior to 2.3.0, shouldn't be a big deal to remove that in 3.0.0 in exchange for better backtraces

Fishrock123 commented 3 years ago

It'd be neat to have this at least as a optional feature