Print more information for TLS errors #143
Conversation
When `.to_string()` is invoked on a the crate-wide Error enum,
the error message using #[error("...")] is displayed.
Some of those wrapped internal enums also contain, possibly more
helpful, error messages. This will include the string gathered by
`.to_string()` from the encapsulated enum in the main error message.
See also
squidowl/halloy#143
|
I've created an upstream PR as the code was much more easy than anticipated :) |
|
Thanks for opening the PR! It's idiomatic to not expose all nested error context in the I'd prefer to see the original approach vs having the maintainer pollute the top-level error display with all context, which may not be suitable for all use-cases of handling this error. So without changing upstream, a good solution might be: let error = match e {
irc::Error::Tls(e) => format!("a TLS error occurred: {e}"),
_ => e.to_string(),
}; |
|
Thank you for your comment & time! Ah, I see – I was just mirroring what other errors in the irc crate do, but I'm a noob when it comes to app design :) From what I can understand, you propose having the "complete" error in the log and the "top level" error in the UI? That would definitely work for me, especially given those TLS errors often even being multiline. |
No problem! I think it's fine to use that new error string in both places. I think it'd be helpful to see it in the UI buffer since it's probably a common error where the context might help. |
The aatxe/irc crate defines a crate-wide Error enum in irc/src/error.rs,
wrapping different errors from various kinds of sources, such as
TLS errors by tls-native. When `.to_string()` is invoked on such an
enum, the error message using `#[error("...")]` is displayed. However,
they wrap the actual Tls error enum inside with its own `.to_string()`
function. In case of such errors, also include this information in our
error message which is much more helpful for debugging.
|
Awesome, thanks for the feedback, I've forced-pushed your requested changes and tested the PR, it works well! |
|
@ljrk0 Looks great, thanks for the contribution 🎉 |
The aatxe/irc crate defines a crate-wide Error enum in irc/src/error.rs, wrapping different errors from various kinds of sources, such as TLS errors by tls-native. When
.to_string()is invoked on such an enum, the error message using#[error("...")]is displayed. However, they wrap the actual Tls error enum inside with its own.to_string()function. In case of such errors, also include this information in our error message which is much more helpful for debugging.Note that perhaps it'd be better to fix this upstream, it seems that the macro can at least also access fields from the enum, as seen here: https://github.com/aatxe/irc/blob/develop/src/error.rs#L76. Maybe something like
with a corresponding field in the enum upstream would work, but my Rust foo is quite limited.