feat(server): convert encoder/helper/echo internals from anyhow to ServerError#1243
Open
Greg Lamberson (glamberson) wants to merge 2 commits intoDevolutions:masterfrom
Open
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
…rverError Second step of the staged migration started in the previous commit. Replaces anyhow construction sites in modules whose internal flow does not pass through the ConnectionHandler::on_disconnected callback (which still takes &anyhow::Error and is the subject of a separate follow-up PR per Devolutions#1209): - encoder/mod.rs: ~15 anyhow! / .context() / bail! sites converted to typed ServerError variants (Encode, Reason, Custom). EncodeError sources go through ServerError::encode (matching ConnectorErrorExt::encode). spawn_blocking JoinError, qoi codec errors, and zstd error codes go through ServerError::custom or ServerError::reason as appropriate. - helper.rs: TLS cert/key loading paths construct ServerError::io for std::io::Error sources, ServerError::reason for missing-key cases, ServerError::custom for x509-cert and PEM parsing errors. Removes the from_anyhow bridge and the inner-fn split introduced in the previous commit. - echo.rs: build_echo_request returns ServerResult, builds errors via ServerError::custom directly. send_request keeps its already- typed Channel and Reason variants. server.rs internals stay on anyhow because they propagate into ConnectionHandler::on_disconnected which still takes &anyhow::Error; that conversion plus the remaining server.rs internal sites land in PR Devolutions#3. The anyhow dep stays in Cargo.toml for now and will be dropped when the public traits (ConnectionHandler, RdpServerDisplay, and RdpServerDisplayUpdates) finish their typed migration.
This was referenced Apr 30, 2026
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
2 participants
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
Second step of the staged migration toward a typed public error story for
ironrdp-server(#1209). Stacks on #1242.Replaces
anyhowconstruction sites in modules whose internal flow does not pass through theConnectionHandler::on_disconnectedcallback (which still takes&anyhow::Errorand is the subject of a separate follow-up PR):encoder/mod.rs: ~15anyhow!/.context()/bail!sites converted to typedServerErrorvariants (Encode,Reason,Custom).EncodeErrorsources go throughServerError::encode(matchingConnectorErrorExt::encode).spawn_blockingJoinError, qoi codec errors, and zstd error codes go throughServerError::customorServerError::reasonas appropriate.helper.rs: TLS cert/key loading paths constructServerError::ioforstd::io::Errorsources,ServerError::reasonfor missing-key cases,ServerError::customforx509-certand PEM parsing errors. Removes thefrom_anyhowbridge and the inner-fn split introduced in feat(server)!: introduce typed ServerError on the public API boundary #1242.echo.rs:build_echo_requestreturnsServerResult, builds errors viaServerError::customdirectly.send_requestkeeps its already-typedChannelandReasonvariants.What this PR does NOT touch
server.rsinternals (the heavy.context()chain inrun_innerandrun_connection_inner) stay onanyhowbecause they propagate intoConnectionHandler::on_disconnected(error: Option<&anyhow::Error>). PR Panic in UserDataHeader::from_buffer due to integer overflow #3 in the staged migration will (a) change that parameter toOption<&ServerError>, and (b) convert the server.rs internal sites in the same change.RdpServerDisplay/RdpServerDisplayUpdatestraits (which returnanyhow::Result) stay unchanged. PR Capset + fuzzing fixes #4 will convert those, drop theanyhowdependency entirely, and complete the migration.Why split this way
server.rsand the display traits are bothanyhow-flavored at the boundary; converting them in a single PR withon_disconnectedkeeps the type story coherent. Splitting them now would either require a temporaryanyhow ↔ ServerErrortwo-way bridge (ugly) or leave the public trait inconsistent with the rest of the crate (worse). Better to ship this batch (encoder + helper + echo all internal-only) and then take server.rs + on_disconnected as one coherent step.Stacking note
Branch is stacked on
feat/server-typed-error(#1242). Rebase onto master after #1242 merges.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-targetscleanTracking: #1209.