fix(connector)!: introduce sspi-free SequenceError

[Benoît Cortier (CBenoit)]

ConnectorErrorKind mixed sequence-level concerns (encode/decode,
negotiation failure, generic reasons) with connect-flow-level ones
(CredSSP's sspi::Error, access-denied), so every Sequence impl and
helper depended on ironrdp_connector::ConnectorError, and through it
on sspi, even though a state machine never touches CredSSP or
access-denied semantics. That coupling is also what blocks extracting
Sequence/MonotonicInstant into a standalone, sspi-free crate
(#1426).

Add SequenceError/SequenceResult (sequence_error.rs) as the
sspi-free error type produced while driving a single PDU state
machine. Sequence::step/step_no_input, every in-workspace
implementor, and single_sequence_step/single_sequence_step_read
now return SequenceResult instead of ConnectorResult.
ConnectorErrorKind narrows to the nested top-level connect-flow
union it always conceptually was: Sequence(SequenceError),
Credssp(sspi::Error), AccessDenied.

ConnectorError and SequenceError are both instantiations of the
foreign generic ironrdp_error::Error<_>, so orphan rules forbid a
direct impl From<SequenceError> for ConnectorError. Each connect/
accept boundary (ironrdp-async, ironrdp-blocking,
ironrdp-acceptor, and downstream consumers) instead performs the
single sanctioned mapping via a new impl ironrdp_error::ErrorMapping<SequenceErrorKind> for ConnectorErrorKind
and the re-exported ResultExt::map_err_as::<ConnectorErrorKind>().
ironrdp_connector::map_sequence_error covers the few call sites that
need to convert an already-produced SequenceError value directly
(not through a Result), e.g. reporting a SequenceError through an
output-event channel.

Sequence/MonotonicInstant stay in ironrdp-connector for now; the
extraction into a standalone crate is the next layer (#1426). No
protocol behavior changes.

Breaking changes, all in ironrdp-connector:

• ConnectorErrorKind's flat Encode/Decode/Reason/General/
  Custom/Negotiation variants are removed; downstream matches must
  go through the new Sequence(SequenceError) variant instead.
• ConnectorErrorExt is removed in favor of SequenceErrorExt on
  SequenceError.
• The general_err!/reason_err!/custom_err! macros now construct
  SequenceError instead of ConnectorError.
• Sequence::step/step_no_input and public helpers such as
  ClientConnector::skip_multitransport return SequenceResult
  instead of ConnectorResult, breaking every external Sequence
  implementor and caller.

Closes #1425

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

Introduces an SSPI-independent sequence error model, preparing the state-machine framework for extraction under #1426.

Changes:

• Adds SequenceError/SequenceResult and migrates all Sequence implementations.
• Nests sequence failures within ConnectorError at connection boundaries.
• Updates downstream error conversion, categorization, and FFI handling.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ffi/src/error.rs	Converts bare and nested sequence errors for FFI.
crates/ironrdp-web/src/network_client.rs	Maps network failures into connector errors.
crates/ironrdp-web/src/error.rs	Handles nested and direct sequence errors.
crates/ironrdp-vmconnect/src/lib.rs	Adds sequence-to-connector boundary mappings.
crates/ironrdp-tokio/src/reqwest.rs	Maps transport failures into connector errors.
crates/ironrdp-daemon/src/daemon.rs	Updates the connection-failure test fixture.
crates/ironrdp-connector/src/sequence_error.rs	Defines the new sequence error API.
crates/ironrdp-connector/src/macros.rs	Makes error macros produce sequence errors.
crates/ironrdp-connector/src/license_exchange.rs	Migrates licensing state-machine errors.
crates/ironrdp-connector/src/lib.rs	Revises core error types and mapping APIs.
crates/ironrdp-connector/src/credssp.rs	Maps non-sequence failures at CredSSP boundaries.
crates/ironrdp-connector/src/connection.rs	Migrates the client connector sequence.
crates/ironrdp-connector/src/connection_finalization.rs	Migrates finalization sequence errors.
crates/ironrdp-connector/src/connection_activation.rs	Migrates activation sequence errors.
crates/ironrdp-connector/src/channel_connection.rs	Migrates channel-connection sequence errors.
crates/ironrdp-client/src/rdp.rs	Updates client-side mappings and failure events.
crates/ironrdp-blocking/src/connector.rs	Maps blocking sequence errors at boundaries.
crates/ironrdp-async/src/framed.rs	Returns sequence errors from framed helpers.
crates/ironrdp-async/src/connector.rs	Maps asynchronous sequence errors at boundaries.
crates/ironrdp-activex/src/control.rs	Handles nested sequence error categories.
crates/ironrdp-acceptor/src/util.rs	Migrates acceptor encoding helpers.
crates/ironrdp-acceptor/src/lib.rs	Maps acceptor sequence errors at boundaries.
crates/ironrdp-acceptor/src/finalization.rs	Migrates acceptor finalization errors.
crates/ironrdp-acceptor/src/credssp.rs	Updates acceptor CredSSP mappings.
crates/ironrdp-acceptor/src/connection.rs	Migrates the main acceptor sequence.
crates/ironrdp-acceptor/src/channel_connection.rs	Migrates acceptor channel errors.

[github-actions]

Mechanical error-type split: ConnectorErrorKind's flat Encode/Decode/Reason/General/Custom/Negotiation variants move into a new sspi-free SequenceError/SequenceErrorKind, and every Sequence::step/step_no_input implementor plus the async/blocking/acceptor boundary functions are updated to produce SequenceResult and convert to ConnectorResult via the pre-existing ErrorMapping/ResultExt::map_err_as mechanism (unchanged, not part of this diff). No encode/decode routine, PDU field, or state transition is touched; all call sites that need the coarser type explicitly convert. One display-fidelity regression survives the refactor and one API-scope question remains open.

Protocol analysis: accepted — Independently walked the diff: every hunk is a rename (ConnectorError::decode/encode -> SequenceError::decode/encode), a return-type change (ConnectorResult -> SequenceResult) on Sequence impls and their private helpers, or an added .map_err_as::<ConnectorErrorKind>() at a connect/accept boundary. No decode/encode call, PDU field, capability value, or state-machine transition is altered — only the error type flowing out of unchanged decode/encode calls changes shape. The handoff's 'none' protocol relevance is correct.

1. non_blocking / medium — crates/ironrdp-connector/src/lib.rs
   Before this PR, ConnectorErrorKind's plain Display inlined the specific failure ("decode error", "negotiation failure: {failure}", etc.) at the top level. Now ConnectorErrorKind::Sequence(_) always displays the generic "sequence error"; the specific SequenceErrorKind detail only surfaces one level down via .source()/.report(). Call sites that already use .report() (ffi/src/error.rs, ironrdp-viewer) or match on .kind() (ironrdp-activex) are unaffected, but crates/ironrdp-web/src/error.rs's backtrace() (unchanged by this PR) does format!("{:#}", anyhow::Error::new(connector_error)), which only invokes the outer Display (anyhow's Display doesn't walk the source chain) and will now always render "sequence error" instead of the previously distinguishable message for any Sequence-produced failure, e.g. a web-client connection failing during X.224 negotiation or PDU decode now surfaces "[sequence error @ file:line] sequence error" instead of the previously distinguishing "negotiation failure: ..." or "decode error", degrading the web client's connection-failure diagnostics.
2. question / low — crates/ironrdp-connector/src/license_exchange.rs
   LicenseCache::get_license/store_license still return ConnectorResult (untouched by this PR) even though the only place they are called is inside LicenseExchangeSequence::step, which the PR just switched to SequenceResult. The call sites now bridge back with .map_err(|e| custom_err!("get_license"/"store_license", e)), wrapping a full ConnectorError (still type-capable of holding an sspi::Error, even though a license cache will never produce one) as the source of a SequenceError, so this Sequence impl's sspi-freedom is only skin-deep at its LicenseCache boundary. Was leaving LicenseCache's trait error type as ConnectorResult a deliberate deferral (e.g. to avoid another breaking change to an external-implementor trait in the same PR), or an oversight given the trait is exclusively invoked from a Sequence::step context this PR otherwise made sspi-free?

[github-actions]

Well-scoped error-type split: SequenceError/SequenceResult now cover Sequence::step and its helpers; ConnectorErrorKind narrows to Sequence/Credssp/AccessDenied; boundary crossings use the existing ErrorMapping mechanism via .map_err_as::<ConnectorErrorKind>() or the new map_sequence_error() helper. Traced changed call sites plus un-diffed FFI/web files relying on new From<SequenceError> impls; conversions are complete, no leftover ConnectorErrorExt usage or compile gaps. No protocol/wire-format logic touched, matching the handoff. Two defects: map_sequence_error() lacks #[track_caller] (unlike sibling ErrorMapping::map_error), so its production call site (clipboard init failure) reports a wrong error location to ActiveX host tracing. Also ErrorMapping::map_error hardcodes "sequence error" as outer context for every conversion, so non-Sequence failures (TCP connect, TLS upgrade, CredSSP parsing) show a misleading prefix.

Protocol analysis: accepted — Independently verified: every decode/encode call site only swapped its error constructor from ConnectorError::decode/encode to SequenceError::decode/encode with identical arguments, no PDU field, state-transition, or negotiation-branch logic changed, and negotiation handling is preserved verbatim (SequenceError::negotiation mirrors the old ConnectorError::new(..., Negotiation(...))). The flagged uncertainty (license_exchange.rs get_license/store_license now wrap failures via explicit custom_err! instead of implicit propagation) is real but confirmed to only affect error text/nesting, not license-exchange protocol logic.

1. non_blocking / low — crates/ironrdp-connector/src/lib.rs
   map_sequence_error() forwards to ErrorMapping::map_error (which is #[track_caller] and ultimately calls ConnectorError::new, also #[track_caller]) but map_sequence_error itself is not annotated #[track_caller]. Since #[track_caller] location propagation stops at the first non-annotated frame, every ConnectorError produced through map_sequence_error will report its location as this line in lib.rs rather than the real call site. The one production use (crates/ironrdp-client/src/rdp.rs, Windows clipboard backend init failure sent as RdpOutputEvent::ConnectionFailure) feeds into ironrdp-activex's trace_connection_failure, which reads error.location() specifically to log "file:line" for host diagnostics - that trace will now always point at ironrdp-connector/src/lib.rs instead of the clipboard init failure site.
2. non_blocking / low — crates/ironrdp-connector/src/lib.rs
   ErrorMapping::map_error always wraps with the fixed context "sequence error" (confirmed by the new testsuite-core test, which shows the string "[sequence error] [Capabilities Exchange] reason: ..."). Because general_err!/reason_err!/custom_err! now build a SequenceError even at call sites that are not part of any Sequence::step (e.g. TCP connect and TLS upgrade in ironrdp-client/src/rdp.rs, TsRequest (de)serialization in ironrdp-connector/src/credssp.rs, KDC network I/O in ironrdp-tokio/src/reqwest.rs), every one of those unrelated failures is now displayed with a "[sequence error]" prefix ahead of its real, specific context string. No information is lost (the specific context still appears one level in), but the added generic label is inaccurate for the majority of these call sites and adds noise to user/operator-facing error text across most of the connector crate's consumers.
3. question / low — crates/ironrdp-connector/src/license_exchange.rs
   LicenseExchangeSequence::step was migrated to the sspi-free SequenceResult, but the LicenseCache plugin trait it calls into (get_license/store_license) still returns ConnectorResult, i.e. Result<_, ConnectorError>, whose ConnectorErrorKind enum carries an sspi::Error variant. Any external LicenseCache implementor therefore still needs ConnectorErrorKind (and transitively sspi) in scope, and the call sites now wrap that ConnectorError as the source of a new SequenceError via custom_err! rather than propagating it directly. Is leaving LicenseCache on ConnectorResult intentional pending the Sequence/MonotonicInstant extraction tracked in #1426, or should it also move to SequenceResult now for consistency with the PR's stated goal that a Sequence impl and its helpers never need ConnectorError?