eyre icon indicating copy to clipboard operation
eyre copied to clipboard

feat: implement serialize for report

Open thewh1teagle opened this issue 1 year ago • 9 comments

I use eyre with tauri. When returning a result from it, it requires that the error implements Serialize (as it returns it to the browser). So instead, I had to return std::result::Result<(), String> (which of course implements it) and map many errors to a string each time:

#[command]
fn hello() -> Result<(), String> {
  do_something().map_err(|e| e.to_string())?;
}

With this feature, I can now return an eyre result and propagate almost any error.

#[command]
fn hello() -> Result<()> {
  do_something()?;
}

thewh1teagle avatar Apr 23 '24 00:04 thewh1teagle

Turns out that by default err.to_string() in eyre returns from fmt::Display, and it doesn't include the backtrace in the error string, and the final error string (which returns to the browser in tauri) is useless.

I'm thinking of a way to include the backtrace conditionally in the serializer from eyre, What do you think about adding serialize_backtrace feature which will be used as:

use serde::{ser::Serializer, Serialize};
use crate::Report;

impl Serialize for Report {
    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        if cfg!(feature = "serialize_backtrace") {
            serializer.serialize_str(format!("{:?}", self).as_ref()) // include backtrace
        } else {
            serializer.serialize_str(self.to_string().as_ref())
        }
    }
}

thewh1teagle avatar Apr 29 '24 21:04 thewh1teagle

Sounds even better. I'm not sure how it can work with a custom handler (or how Eyre currently works with custom handlers).

Anyway, I think that should be another feature, and the default serializer should be just like the default error, with a backtrace (currently, the format in the PR doesn't include a stack trace).

thewh1teagle avatar May 02 '24 15:05 thewh1teagle

Clippy is failing on master due to introducing additional lints in a new version, I'll fix it up, so don't worry about it.

ten3roberts avatar May 13 '24 16:05 ten3roberts

This is great! I'm working on another tauri project, so this would be helpful.

Since color_eyre re-exports eyre, could you add serialize as a feature to that as well?

jsimonrichard avatar May 31 '24 21:05 jsimonrichard

Whats the status of this? would really appreciate this.

MordechaiHadad avatar Jun 25 '24 10:06 MordechaiHadad

Whats the status of this? would really appreciate this.

There seems to be no consensus on the structural serialization format.

Serializing the Debug representation is easily done by crate users by implementing Serialize on a newtype wrapper.

The real interesting serialization would include information internal to eyre::Report, such as the error chain (backtrace) and stack trace for each error in the chain - where available.

It seems to be that this needs some more design work before we can settle on an implementation.

The serialization output would be part of the public API. Thus, if we serialize a Debug representation of the eyre::Report, we are bound to sticking with that.

LeoniePhiline avatar Jun 28 '24 11:06 LeoniePhiline

Whats the status of this? would really appreciate this.

There seems to be no consensus on the structural serialization format.

Serializing the Debug representation is easily done by crate users by implementing Serialize on a newtype wrapper.

The real interesting serialization would include information internal to eyre::Report, such as the error chain (backtrace) and stack trace for each error in the chain - where available.

It seems to be that this needs some more design work before we can settle on an implementation.

The serialization output would be part of the public API. Thus, if we serialize a Debug representation of the eyre::Report, we are bound to sticking with that.

Exactly.

I think best approach would be to add serialization to the handler (I.e; color-eyre), because the handler is the one who knows about things like spans, backtrace, etc, and it may be different based on handler what information is available.

A new method to the Handler trait would with a default-noop impl would then be exposed to access this handler-specific serialization.

To start with, we can add it to color-eyre and see how a structural serialization fairs there (basically struct-like serde with spantrace, backtrace, and error chain) and go from there

ten3roberts avatar Jun 28 '24 13:06 ten3roberts

Would it be possible to add initial support for serialization first? This way, users won't need to implement their own types and deal with the overhead of getting serialization for errors. This PR has been pending for two months, and addressing this now would help reduce the waiting time for users. Once we have this minimal support in place, we can explore various improvements in future PRs/issues. Could you please clarify the benefits of waiting this long before merging?

thewh1teagle avatar Jun 28 '24 14:06 thewh1teagle

any update on this? would love this feature. thanks for your work on it.

dionysuzx avatar Sep 13 '24 22:09 dionysuzx