refactor: derive Display and Error via thiserror in pdu, connector, session#1260
Closed
Greg Lamberson (glamberson) wants to merge 1 commit into
Closed
Conversation
…ession Refs Devolutions#1257. Part of the thiserror migration proposed in Devolutions#1257. The first proof-of-concept (Devolutions#1258, mstsgu) verified the byte-identical Display approach. This PR applies the same mechanical pattern to the three std-only crates with hand-written *ErrorKind impls atop ironrdp-error. ironrdp-core is handled in a separate PR because of its no_std-aware feature wiring. ### Per-crate notes - ironrdp-pdu: thiserror was already pinned at "2.0" for some internal types. PduErrorKind is now derived. Hand-written Display + Error impls removed. - ironrdp-connector: adds `thiserror = "2"` direct dep. ConnectorErrorKind is derived. Tuple variants (Encode, Decode, Credssp, Negotiation) receive `#[source]` attributes so the source chain that the prior hand-written `core::error::Error::source()` impl established is preserved. - ironrdp-session: same pattern as connector. SessionErrorKind is derived with `#[source]` on Pdu, Encode, Decode tuple variants. Removes the now-unused `use core::fmt;` import. ### Public API impact None. The `pub type FooError = ironrdp_error::Error<FooErrorKind>` aliases and the `*ErrorExt` traits are unchanged. Consumers see no source-level break. ### Display byte-identity verification Verified via cargo expand. Each derived impl calls `formatter.write_str()` or `write_fmt(format_args!())` with the same literal/arguments as the prior hand-written code. Unit variants emit identical write_str calls; tuple variants with formatted source (e.g. Reason(String), Negotiation) emit `format_args!("prefix: {0}", _0.as_display())` which calls Display on the inner field, identical to the prior `write!(f, "prefix: {field_name}")`. ### xtask verification - `cargo xtask check fmt`: clean - `cargo xtask check lints`: clean - `cargo xtask check locks`: clean (root Cargo.lock updated and included) - `cargo xtask check typos`: clean - `cargo xtask check tests`: clean
Contributor
Author
|
Closing per #1257 (#1257 (comment)). Change 1's premise is withdrawn after the Core Tier proc-macro invariant correction. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1257. Replaces #1259 (closed for split).
Part of the thiserror migration proposed in #1257. The first proof-of-concept (#1258, mstsgu) verified the byte-identical Display approach. This PR applies the same mechanical pattern to the three std-only crates with hand-written
*ErrorKindimpls atopironrdp-error.ironrdp-coreis handled in a separate PR (incoming) because of its no_std-aware feature wiring.Per-crate notes
ironrdp-pdu:thiserrorwas already pinned at"2.0"for some internal types.PduErrorKindis now derived. Hand-writtenDisplayandErrorimpls removed.ironrdp-connector: addsthiserror = "2"as a direct dep.ConnectorErrorKindis derived. Tuple variants (Encode,Decode,Credssp,Negotiation) receive#[source]attributes so the source chain that the prior hand-writtencore::error::Error::source()impl established is preserved.ironrdp-session: same pattern as connector.SessionErrorKindis derived with#[source]onPdu,Encode,Decodetuple variants. Removes the now-unuseduse core::fmt;import.Public API impact
None. The
pub type FooError = ironrdp_error::Error<FooErrorKind>aliases and the*ErrorExttraits are unchanged. Consumers see no source-level break.Display byte-identity verification
Verified via
cargo expand. Each derived impl callsformatter.write_str()orwrite_fmt(format_args!())with the same literal/arguments as the prior hand-written code. Unit variants emit identicalwrite_strcalls; tuple variants with formatted source (e.g.Reason(String),Negotiation) emitformat_args!("prefix: {0}", _0.as_display())which callsDisplayon the inner field, identical to the priorwrite!(f, "prefix: {field_name}").xtask verification
cargo xtask check fmt: cleancargo xtask check lints: cleancargo xtask check locks: clean (rootCargo.lockupdated and included)cargo xtask check typos: cleancargo xtask check tests: cleanDiff size
6 files changed, +31 −86 (net −55 lines of code removed).