Repository navigation
Implement Serialize/Deserialize on error types #796
Description
The TODO is saying: instead of a string repr of the error it should be any error type that plays nicely with serde. In the future we may want to take action on why a session closed. However its a TODO because we dont impl serde::serialize/deserialize on our error types.
Originally posted by @0xBEEFCAF3 in #760 (comment)
Activity
GideonBature commentedon Jun 24, 2025
Will this be applied to all the error types in the rust-payjoin crate? irrespective of where they appear?
arminsabouri commentedon Jun 25, 2025
Most likely no. Only on the error types that need to be persisted in a session event
spacebear21 commentedon Sep 18, 2025
Related issue: #793
arminsabouri commentedon Sep 19, 2025
Can we close this? #1060 attempts to fix the root problem (#793)
spacebear21 commentedon Sep 19, 2025
I'd wait until #1060 actually gets merged/demonstrates that the problem is solved.
DanGould commentedon Sep 21, 2025
agree that we don't actually want to use the solution proposed here and also that we can wait for the solution to merge to close this
arminsabouri commentedon Oct 3, 2025
Revisiting this. #1060 is merged.
The only remaining problem may be that we lost some visibility into fatal errors that are non-replyable.
The
Closedvariant of session outcome can encapsulate a stringified error.DanGould commentedon Oct 3, 2025
I think the spirit of this was to be able to store and replay errors. I don't think each error type actually neeeds to be Serialize/Deserialize. How much visibility into fatal errors did we lose? As long as UIs get a message I think that's sufficient even if stack traces and debug logs would be nice to have.
arminsabouri commentedon Oct 3, 2025
AFIAIC we don't log the error message if the error is not replayble. @spacebear21 please correct me if I am wron
g
DanGould commentedon Oct 3, 2025
A log::debug for good measure might make troubleshooting more robust but it's not 1.0rc blocking imo
spacebear21 commentedon Oct 3, 2025
I think @arminsabouri is talking about logging in the session event log while @DanGould is talking about debug logging. I agree log::debug in rust-payjoin might make troubleshooting more robust instead of relying on implementers to log it explicitly. Re: logging fatal error messages in the SEL (#1144) I don't think we lose much by not doing that. AFAICT the only fatal errors that aren't replyable relate to v2 response processing, so storing those details in SEL doesn't seem critical.
DanGould commentedon Oct 3, 2025
Sounds like log::error would work there, too.
All that really needs to be done in UI is to display that the thing errored fatally and won't resume. A message would be more ergonomic but the whole error w/ stack trace is only useful for debugging, not for users
arminsabouri commentedon Oct 3, 2025
Agreed. The message encapsulated wouldnt be a stack trace. Just the string representation of the child error message.