Implement Serialize/Deserialize on error types

[spacebear21]

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)

[GideonBature]

Will this be applied to all the error types in the rust-payjoin crate? irrespective of where they appear?

[arminsabouri]

Will this be applied to all the error types in the rust-payjoin crate? irrespective of where they appear?

Most likely no. Only on the error types that need to be persisted in a session event

[spacebear21]

Related issue: #793

[arminsabouri]

Can we close this? #1060 attempts to fix the root problem (#793)

[spacebear21]

I'd wait until #1060 actually gets merged/demonstrates that the problem is solved.

[DanGould]

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]

Revisiting this. #1060 is merged.
The only remaining problem may be that we lost some visibility into fatal errors that are non-replyable.
The Closed variant of session outcome can encapsulate a stringified error.

[DanGould]

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]

How much visibility into fatal errors did we lose?

AFIAIC we don't log the error message if the error is not replayble. @spacebear21 please correct me if I am wron
g

[DanGould]

A log::debug for good measure might make troubleshooting more robust but it's not 1.0rc blocking imo

[spacebear21]

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]

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