Devolutions / Devolutions/IronRDP

Introduce `SequenceError`; make the `Sequence` chain sspi-free

Open
#1,425 0 comments 0 reactions 0 assignees View on GitHub
kind/technical-debt scope/core
Dominant language
Rust
Stars
3.2k
Forks
275
Avg merge
1d 11h
Merged PRs (30d)
189

Description

The `Sequence` chain produces only sspi-free error kinds (`general_err!`, `reason_err!`, `custom_err!`, one `Negotiation`). `sspi::Error` only appears in the separately-driven CredSSP path. Split the error so the `Sequence` contract is sspi-free, in preparation for moving the framework out (#1426) and injecting CredSSP (#1430). Pure error refactor — no crate move yet.

### Scope
- [ ] Add `SequenceError` (kinds: `Encode`, `Decode`, `Reason`, `General`, `Custom`, `Negotiation(NegotiationFailure)`), sspi-free.
- [ ] Make the `Sequence` trait and the whole PDU state machine return `SequenceError`.
- [ ] `ConnectorError` is the **top-level connect error only** (returned by `connect_*`), *not* the `Sequence` error. In this PR, slim it to the *nested* connect-flow union `From` + `Credssp(sspi::Error)` + `AccessDenied` (the latter two originate in the separately-driven `perform_credssp_step`, never in a `Sequence` step) and **keep the name** for now. Retiring the global `ConnectorError` in favor of the focused `ConnectError { Sequence(SequenceError), Credssp(Box) }` happens in #1430, when the credssp payload changes anyway (see the meta's error model). Use the *nested* shape (`Sequence(SequenceError)`, not flattening its kinds up) so it lines up with the eventual `ConnectError` and doesn't duplicate `SequenceError`'s kinds — this nests `ironrdp-web`'s `Negotiation` match under `Sequence(..)`, so web is in this PR's footprint.
- [ ] `single_sequence_step`/`_read`/`_write` return `SequenceError`, **not** `ConnectorError` (their only failure source is `sequence.step()`; CredSSP is driven separately, so routing through `ConnectorError` there would fabricate a `Credssp` variant that can never occur). Framed read/write IO errors fold into `SequenceError::Custom` (boxed source — keeps `ironrdp-sequence` transport-agnostic). The `SequenceError → ConnectorError` conversion happens once, at the `connect_*` boundary, via `?`, alongside `perform_credssp_step`'s error.
- [ ] Same-PR consumer updates (compile-forcing): every `impl Sequence` in `ironrdp-acceptor` (`channel_connection.rs`, `connection.rs`, `finalization.rs`) returns `SequenceError`; the `connect_*` functions in `ironrdp-async`/`ironrdp-blocking` do the single boundary map. Reactivation loops in `ironrdp-client`/`ironrdp-web`/ffi call `single_sequence_step_read`: transparent if they propagate via a generic error (`anyhow`/`Box`), but need touching if they match `ConnectorError` at that call site.

### Acceptance criteria
- `ironrdp-connector`, `ironrdp-async`, `ironrdp-blocking`, `ironrdp-acceptor`, `ironrdp-web` build with equivalent error semantics.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the Sequence trait and PDU state machine, then trace single_sequence_step, single_sequence_step_read, and single_sequence_step_write. Update the Sequence implementations in ironrdp-acceptor's channel_connection.rs, connection.rs, and finalization.rs, plus the connect_* boundaries and reactivation call sites named in the issue. Done means the listed crates build with SequenceError separated from the top-level ConnectorError and equivalent error semantics preserved.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.