From 4365a85b5ef1142e55f1f65803a30da485e2fb4b Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Fri, 15 May 2026 14:16:05 -0500 Subject: [PATCH] fix(rdpsnd-native)!: replace anyhow with typed RdpsndNativeError Brings ironrdp-rdpsnd-native into line with the workspace convention from ARCHITECTURE.md that libraries provide concrete error types rather than anyhow::Result. Defines RdpsndNativeErrorKind with four variants covering the failure modes in DecodeStream::new (UnsupportedFormat, OpusInit, AudioDevice, StreamBuild). Display and core::error::Error impls are hand-rolled per the post-#1264 pattern in ironrdp-pdu. DecodeStream::new now returns RdpsndNativeResult instead of anyhow::Result. Foreign error sources (opus2::Error, cpal::DefaultStreamConfigError, cpal::BuildStreamError) are attached via Error::with_source so the cause chain remains reachable through core::error::Error::source(). anyhow moves from [dependencies] to [dev-dependencies] for the cpal example binary. The internal log of construction failures switches from "{e:#}" to e.report() to preserve cause-chain output now that anyhow's alternate-format chain printing is no longer involved. Signed-off-by: Greg Lamberson --- Cargo.lock | 1 + crates/ironrdp-rdpsnd-native/Cargo.toml | 3 +- crates/ironrdp-rdpsnd-native/src/cpal.rs | 47 +++++++++++++++-------- crates/ironrdp-rdpsnd-native/src/error.rs | 39 +++++++++++++++++++ crates/ironrdp-rdpsnd-native/src/lib.rs | 11 ++++-- 5 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 crates/ironrdp-rdpsnd-native/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index f8a4364fa..29786519d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2791,6 +2791,7 @@ dependencies = [ "anyhow", "bytemuck", "cpal", + "ironrdp-error", "ironrdp-rdpsnd", "opus2", "tracing", diff --git a/crates/ironrdp-rdpsnd-native/Cargo.toml b/crates/ironrdp-rdpsnd-native/Cargo.toml index 928b543f6..f596b47e7 100644 --- a/crates/ironrdp-rdpsnd-native/Cargo.toml +++ b/crates/ironrdp-rdpsnd-native/Cargo.toml @@ -20,14 +20,15 @@ default = ["opus"] opus = ["dep:opus2", "dep:bytemuck"] [dependencies] -anyhow = "1" bytemuck = { version = "1.24", optional = true } cpal = "0.17" +ironrdp-error = { path = "../ironrdp-error", version = "0.1", features = ["std"] } # public ironrdp-rdpsnd = { path = "../ironrdp-rdpsnd", version = "0.7" } # public opus2 = { version = "0.4", optional = true, features = ["bundled"] } tracing = { version = "0.1", features = ["log"] } [dev-dependencies] +anyhow = "1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } [lints] diff --git a/crates/ironrdp-rdpsnd-native/src/cpal.rs b/crates/ironrdp-rdpsnd-native/src/cpal.rs index ef338105d..4f9236cb7 100644 --- a/crates/ironrdp-rdpsnd-native/src/cpal.rs +++ b/crates/ironrdp-rdpsnd-native/src/cpal.rs @@ -5,13 +5,15 @@ use std::sync::Arc; use std::sync::mpsc::{self, Receiver, Sender}; use std::thread::{self, JoinHandle}; -use anyhow::{Context as _, bail}; use cpal::traits::{DeviceTrait as _, HostTrait as _}; use cpal::{SampleFormat, Stream, StreamConfig}; +use ironrdp_error::bail; use ironrdp_rdpsnd::client::RdpsndClientHandler; use ironrdp_rdpsnd::pdu::{AudioFormat, PitchPdu, VolumePdu, WaveFormat}; use tracing::{debug, error, info, warn}; +use crate::error::{RdpsndNativeError, RdpsndNativeErrorKind, RdpsndNativeResult}; + #[derive(Debug)] pub struct RdpsndBackend { // Unfortunately, Stream is not `Send`, so we move it to a separate thread. @@ -91,7 +93,7 @@ impl RdpsndClientHandler for RdpsndBackend { let stream = match DecodeStream::new(&format, rx) { Ok(stream) => stream, Err(e) => { - error!(error = format!("{e:#}")); + error!(error = %e.report()); return; } }; @@ -138,7 +140,7 @@ pub struct DecodeStream { } impl DecodeStream { - pub fn new(rx_format: &AudioFormat, mut rx: Receiver>) -> anyhow::Result { + pub fn new(rx_format: &AudioFormat, mut rx: Receiver>) -> RdpsndNativeResult { let mut dec_thread = None; match rx_format.format { #[cfg(feature = "opus")] @@ -146,10 +148,15 @@ impl DecodeStream { let chan = match rx_format.n_channels { 1 => opus2::Channels::Mono, 2 => opus2::Channels::Stereo, - _ => bail!("unsupported #channels for Opus"), + _ => bail!( + "unsupported channel count for Opus", + RdpsndNativeErrorKind::UnsupportedFormat, + ), }; let (dec_tx, dec_rx) = mpsc::channel(); - let mut dec = opus2::Decoder::new(rx_format.n_samples_per_sec, chan)?; + let mut dec = opus2::Decoder::new(rx_format.n_samples_per_sec, chan).map_err(|e| { + RdpsndNativeError::new("creating Opus decoder", RdpsndNativeErrorKind::OpusInit).with_source(e) + })?; dec_thread = Some(thread::spawn(move || { while let Ok(pkt) = rx.recv() { let nb_samples = match dec.get_nb_samples(&pkt) { @@ -181,23 +188,31 @@ impl DecodeStream { rx = dec_rx; } WaveFormat::PCM => {} - _ => bail!("audio format not supported"), + _ => bail!( + "matching server-requested wave format", + RdpsndNativeErrorKind::UnsupportedFormat, + ), } let sample_format = match rx_format.bits_per_sample { 8 => SampleFormat::U8, 16 => SampleFormat::I16, - _ => { - bail!("only PCM 8/16 bits formats supported"); - } + _ => bail!( + "only PCM 8/16 bit formats supported", + RdpsndNativeErrorKind::UnsupportedFormat, + ), }; let host = cpal::default_host(); - let device = host.default_output_device().context("no default output device")?; - let _supported_configs_range = device - .supported_output_configs() - .context("no supported output config")?; - let default_config = device.default_output_config()?; + let device = host + .default_output_device() + .ok_or_else(|| RdpsndNativeError::new("no default output device", RdpsndNativeErrorKind::AudioDevice))?; + let _supported_configs_range = device.supported_output_configs().map_err(|e| { + RdpsndNativeError::new("no supported output configs", RdpsndNativeErrorKind::AudioDevice).with_source(e) + })?; + let default_config = device.default_output_config().map_err(|e| { + RdpsndNativeError::new("default output config", RdpsndNativeErrorKind::AudioDevice).with_source(e) + })?; debug!(?default_config); let mut rx = RxBuffer::new(rx); @@ -219,7 +234,9 @@ impl DecodeStream { |error| error!(%error), None, ) - .context("failed to setup output stream")?; + .map_err(|e| { + RdpsndNativeError::new("building cpal output stream", RdpsndNativeErrorKind::StreamBuild).with_source(e) + })?; Ok(Self { _dec_thread: dec_thread, diff --git a/crates/ironrdp-rdpsnd-native/src/error.rs b/crates/ironrdp-rdpsnd-native/src/error.rs new file mode 100644 index 000000000..1c84db69c --- /dev/null +++ b/crates/ironrdp-rdpsnd-native/src/error.rs @@ -0,0 +1,39 @@ +//! Typed error types for `ironrdp-rdpsnd-native`. + +/// Categorises failures in `ironrdp-rdpsnd-native` operations. +/// +/// Bug-shaped conditions are intentionally absent: misuse of this crate's +/// public API should panic or trip `debug_assert!`, not return `Err`. +#[derive(Debug)] +#[non_exhaustive] +pub enum RdpsndNativeErrorKind { + /// Server requested an audio format outside the supported set (wave + /// format, channel count, or bit depth). + UnsupportedFormat, + /// The Opus decoder failed to initialise. Source carries the underlying + /// `opus2::Error` when available. + OpusInit, + /// No usable audio output device or no supported output configuration + /// for the requested format. Source carries the underlying `cpal` error + /// when available. + AudioDevice, + /// The `cpal` output stream could not be built. Source carries the + /// underlying `cpal::BuildStreamError`. + StreamBuild, +} + +impl core::fmt::Display for RdpsndNativeErrorKind { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::UnsupportedFormat => write!(f, "unsupported audio format"), + Self::OpusInit => write!(f, "Opus decoder initialisation"), + Self::AudioDevice => write!(f, "audio output device"), + Self::StreamBuild => write!(f, "output audio stream build"), + } + } +} + +impl core::error::Error for RdpsndNativeErrorKind {} + +pub type RdpsndNativeError = ironrdp_error::Error; +pub type RdpsndNativeResult = Result; diff --git a/crates/ironrdp-rdpsnd-native/src/lib.rs b/crates/ironrdp-rdpsnd-native/src/lib.rs index 04a3b3cfa..5d2c3d3c8 100644 --- a/crates/ironrdp-rdpsnd-native/src/lib.rs +++ b/crates/ironrdp-rdpsnd-native/src/lib.rs @@ -1,7 +1,12 @@ #![cfg_attr(doc, doc = include_str!("../README.md"))] #![doc(html_logo_url = "https://cdnweb.devolutions.net/images/projects/devolutions/logos/devolutions-icon-shadow.svg")] - -#[cfg(test)] -use tracing_subscriber as _; +// `anyhow` and `tracing-subscriber` are dev-deps used only by the `cpal` +// example binary, but `unused_crate_dependencies` still flags them on the +// lib target. The `[lib] test = false` setting makes a `#[cfg(test)]` +// workaround dead code, so the suppression has to apply unconditionally. +#![allow(unused_crate_dependencies)] pub mod cpal; +pub mod error; + +pub use error::{RdpsndNativeError, RdpsndNativeErrorKind, RdpsndNativeResult};