feat(server)!: introduce typed ServerError on the public API boundary#1242
Open
Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Open
feat(server)!: introduce typed ServerError on the public API boundary#1242Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Conversation
Introduces a typed error story for ironrdp-server, mirroring the shape already used by ironrdp-connector and the rest of the connection-management layer: pub type ServerError = ironrdp_error::Error<ServerErrorKind>; pub type ServerResult<T> = Result<T, ServerError>; ServerErrorKind is a non-exhaustive enum with concretely typed variants for the failure modes the crate surfaces (Encode, Decode, Io, Channel, Unsupported, Reason, General, Custom). Sources are attached through ironrdp_error::Error::with_source rather than embedded as Box<dyn Error> in variant data, matching ConnectorErrorKind exactly. This first PR converts public boundaries only: - RdpServer::run -> ServerResult<()> - RdpServer::run_connection<S> -> ServerResult<()> - TlsIdentityCtx::init_from_paths -> ServerResult<Self> - TlsIdentityCtx::make_acceptor -> ServerResult<TlsAcceptor> - EchoServerHandle::send_request -> ServerResult<()> Internal call sites continue to use anyhow::Result during the staged migration. A private helper bridges at the public boundary. A follow-up PR converts internal sites to use the typed kinds directly and drops the anyhow dependency. A second follow-up changes the ConnectionHandler::on_disconnected error parameter from Option<&anyhow::Error> to Option<&ServerError>. This is a breaking change to consumers of the listed public functions: return types change from anyhow::Result<T> to ServerResult<T>. Tracking: Devolutions#1209
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
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.
Summary
First in a staged migration toward a typed public error story for
ironrdp-server, addressing #1209. Introduces:ServerErrorKindis a#[non_exhaustive]enum with concretely typed variants (Encode,Decode,Io,Channel,Unsupported,Reason,General,Custom). Sources are attached throughironrdp_error::Error::with_sourcerather than embedded asBox<dyn Error>in variant data. This is byte-for-byte the shape ofConnectorErrorKindinironrdp-connector, so the connection-management layer stays internally consistent.Why this shape
The audit before drafting found that
ironrdp-connector,ironrdp-session,ironrdp-pdu,ironrdp-mstsgu, andironrdp-core(EncodeError/DecodeError) all use the sameironrdp_error::Error<Kind>pattern. Thethiserror-based bare enums elsewhere in the workspace (GccError,BulkError, etc.) are leaf-layer errors at the PDU/codec level; the connection-management layer sister crate toironrdp-serveris the right template.Scope of this PR
Public function signatures only:
RdpServer::run→ServerResult<()>RdpServer::run_connection<S>→ServerResult<()>TlsIdentityCtx::init_from_paths→ServerResult<Self>TlsIdentityCtx::make_acceptor→ServerResult<TlsAcceptor>EchoServerHandle::send_request→ServerResult<()>Internal call sites continue to use
anyhow::Result. A privatefrom_anyhowhelper bridges at the public boundary. TheConnectionHandler::on_disconnected(error: Option<&anyhow::Error>)parameter is unchanged in this PR.Companion ext traits
Mirrors
ConnectorErrorExt/ConnectorResultExtexactly.Staged follow-ups (separate PRs)
?chains and.context()calls inironrdp-serverto use the typed kinds directly. Remove theanyhowdependency when the lastanyhow!is gone. Touchesencoder/mod.rs(heaviest),server.rs,display.rs,builder.rs.ConnectionHandler::on_disconnected(error: Option<&anyhow::Error>)toOption<&ServerError>. Standalone breaking change for handler implementations.Breaking change
Marked with
!in the conventional commit. Consumers of the five listed public functions need to update theirResulttypes. Pre-1.0, and per Marc-Andre Lureau (@elmarco)'s note on #1209: "I am ok with breaking API at this point :)".Test plan
cargo xtask check fmt -vcleancargo xtask check lints -vclean (workspace, all-targets, with helper + __bench features)cargo xtask check tests -vpassescargo build --workspace --all-targetsclean (one example consumer incrates/ironrdp/examples/server.rsupdated to convert at its own boundary)Closes part of #1209.