Improve Display impl for ConnectorErrorKind

[zmb3]

Prior to this change, a CredSSP connector error would display as [CredSSP] CredSSP - not very helpful.

After the change it looks something like this: [CredSSP] NoAuthenticatingAuthority: Could not contact KDC.

I'm sure there's more we can do here, I've only modified cases where I've been able to induce real errors.

[github-actions[bot]]

Coverage Report :robot: :gear:

Past: Total lines: 33260 Covered lines: 19376 (58.26%)

New: Total lines: 33263 Covered lines: 19376 (58.25%)

Diff: -0.01%

[this comment will be updated automatically]

[CBenoit]

I would however consider this patch an improvement over what we already have:

--- a/crates/ironrdp-connector/src/lib.rs
+++ b/crates/ironrdp-connector/src/lib.rs
@@ -260,7 +260,7 @@ impl fmt::Display for ConnectorErrorKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match &self {
             ConnectorErrorKind::Pdu(_) => write!(f, "PDU error"),
-            ConnectorErrorKind::Credssp(_) => write!(f, "CredSSP"),
+            ConnectorErrorKind::Credssp(e) => write!(f, "{e}"),
             ConnectorErrorKind::Reason(description) => write!(f, "reason: {description}"),
             ConnectorErrorKind::AccessDenied => write!(f, "access denied"),
             ConnectorErrorKind::General => write!(f, "general error"),
@@ -273,7 +273,7 @@ impl std::error::Error for ConnectorErrorKind {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match &self {
             ConnectorErrorKind::Pdu(e) => Some(e),
-            ConnectorErrorKind::Credssp(e) => Some(e),
+            ConnectorErrorKind::Credssp(e) => e.source(),
             ConnectorErrorKind::Reason(_) => None,
             ConnectorErrorKind::AccessDenied => None,
             ConnectorErrorKind::Custom => None,

Giving this:

Connection error: [CredSSP] InvalidToken: CredSSP server returned an error status; status is STATUS_LOGON_FAILURE [0xc000006d]

Actually, I’m not convinced anymore this is an improvement either, because this would prevent the CredSSP error itself from being reached using the source() method. This is sometimes useful when inspecting nested errors (I myself needed to do this in the past for various reasons).

I’ll close as I’m not convinced there is really something we can do to improve anything as-is, but feel free to re-open if you revisit!