-
Notifications
You must be signed in to change notification settings - Fork 66
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
feat: implement serialize for report #168
base: master
Are you sure you want to change the base?
Conversation
Turns out that by default I'm thinking of a way to include the backtrace conditionally in the serializer from eyre, 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())
}
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, serialized errors is something I've made use of too (specifically in games where they are sent over the wire or scripting boundary)
However, do you think we could use a more structural serialization, such as
{
"message": "Something failed",
"caused_by": [ "A failed" ],
"backtrace": [...]
}
Or similar?
This way, we would also support your aforementioned backtrace inclusion, among other data, and allow the error to be picked apart and displayed better on the other end.
What we could do to make it configurable is to add the serialization as a default method on the EyreHandler
so that handler which capture backtraces can show it as well as other handler-specific data like tracing spans.
Example
trait EyreHandler {
...
fn serialize(serializer: &mut dyn erased_serde::Serializer ) -> erased_serde::Result {
... default "good enough" serialization of message and causes
}
}
Sounds even better. 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). |
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. |
This is great! I'm working on another tauri project, so this would be helpful. Since |
Whats the status of this? would really appreciate this. |
There seems to be no consensus on the structural serialization format. Serializing the The real interesting serialization would include information internal to 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 |
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 |
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? |
any update on this? would love this feature. thanks for your work on it. |
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:With this feature, I can now return an
eyre
result and propagate almost any error.