From 8869e991dbcb9153cd6dabd59fc96a6bb2aa061a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 10 Jul 2026 14:03:04 +0200 Subject: [PATCH] Improve error reporting with structured errors and filename context Replace anyhow::Result in fingerprint_file() with a typed FingerprintError enum (OpenFailed, UnrecognizedFormat, NoAudioTrack, DecodeFailed, EmptyFingerprint) that provides user-friendly Display messages. Promote scanner error logging from debug! to warn! so users see which file failed and why at default verbosity. Demote symphonia's internal tracing (which lacks file context) to debug level. Fixes #1 --- src/fingerprint.rs | 71 +++++++++++++++++---- src/main.rs | 6 +- src/scanner.rs | 2 +- tests/integration_test.rs | 131 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+), 15 deletions(-) diff --git a/src/fingerprint.rs b/src/fingerprint.rs index 91814f2..e1fdbcb 100644 --- a/src/fingerprint.rs +++ b/src/fingerprint.rs @@ -4,7 +4,6 @@ //! and [chromaprint-next](https://docs.rs/chromaprint-next) for fingerprint generation. //! The fingerprints are bit-identical to those produced by the reference `fpcalc` tool. -use anyhow::{Context, Result}; use chromaprint::{Algorithm, Fingerprinter}; use std::path::Path; use symphonia::core::audio::{Audio, GenericAudioBufferRef}; @@ -18,6 +17,50 @@ use symphonia::core::meta::MetadataOptions; /// Maximum duration to fingerprint (in seconds). Chromaprint only needs ~120s. pub const DEFAULT_FINGERPRINT_DURATION_SECS: u64 = 120; +/// Categorized reasons why fingerprinting can fail. +#[derive(Debug)] +pub enum FingerprintError { + /// File could not be opened (permissions, missing, locked). + OpenFailed(std::io::Error), + /// Audio format not recognized or file too short/corrupted to probe. + UnrecognizedFormat(String), + /// No audio track found in the container (e.g. video-only file). + NoAudioTrack, + /// Codec is unsupported or audio data is corrupted. + DecodeFailed(String), + /// Fingerprint was empty (file may be silence or too short). + EmptyFingerprint, +} + +impl std::fmt::Display for FingerprintError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::OpenFailed(e) => write!(f, "could not open file ({e})"), + Self::UnrecognizedFormat(detail) => write!( + f, + "not a recognized audio format or file is too short/corrupted ({detail})" + ), + Self::NoAudioTrack => write!(f, "no audio track found in file"), + Self::DecodeFailed(detail) => { + write!(f, "unsupported or corrupted audio codec ({detail})") + } + Self::EmptyFingerprint => write!( + f, + "could not extract audio content (file may be silence or too short)" + ), + } + } +} + +impl std::error::Error for FingerprintError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::OpenFailed(e) => Some(e), + _ => None, + } + } +} + /// Result of fingerprinting a single audio file. #[derive(Debug, Clone)] pub struct FingerprintResult { @@ -35,9 +78,11 @@ pub struct FingerprintResult { /// The fingerprint is generated from the first `max_duration_secs` of audio, /// but `duration_secs` reports the full file duration (critical for deduplication /// since chromaprint can't distinguish files that only differ after the fingerprinted portion). -pub fn fingerprint_file(path: &Path, max_duration_secs: u64) -> Result { - let file = - std::fs::File::open(path).with_context(|| format!("Failed to open: {}", path.display()))?; +pub fn fingerprint_file( + path: &Path, + max_duration_secs: u64, +) -> Result { + let file = std::fs::File::open(path).map_err(FingerprintError::OpenFailed)?; let mss = MediaSourceStream::new(Box::new(file), Default::default()); let mut hint = Hint::new(); @@ -52,19 +97,19 @@ pub fn fingerprint_file(path: &Path, max_duration_secs: u64) -> Result Result Result return Err(e.into()), + Err(e) => return Err(FingerprintError::DecodeFailed(e.to_string())), }; if packet.track_id != track_id { @@ -107,7 +152,7 @@ pub fn fingerprint_file(path: &Path, max_duration_secs: u64) -> Result buf, Err(symphonia::core::errors::Error::DecodeError(_)) => continue, - Err(e) => return Err(e.into()), + Err(e) => return Err(FingerprintError::DecodeFailed(e.to_string())), }; if !finished_fingerprinting { @@ -149,7 +194,7 @@ pub fn fingerprint_file(path: &Path, max_duration_secs: u64) -> Result Result<()> { 2 => "debug", _ => "trace", }; + // Demote symphonia's internal tracing to debug level. Its messages + // (e.g. "probe reached EOF at 4096 bytes") lack file context and confuse users. + // We emit our own warnings with filename and user-friendly explanations instead. + let filter_str = format!("{filter},symphonia_core=debug,symphonia_format_ogg=debug"); tracing_subscriber::fmt() .with_env_filter( - EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(filter)), + EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(&filter_str)), ) .init(); diff --git a/src/scanner.rs b/src/scanner.rs index 41fa952..f7185d0 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -191,7 +191,7 @@ pub fn scan( } } Err(e) => { - debug!("Failed to fingerprint {}: {}", path.display(), e); + warn!("Skipping {}: {}", path.display(), e); ScanResult::Error { path: path.clone(), meta: FileMeta::from_path(path), diff --git a/tests/integration_test.rs b/tests/integration_test.rs index cd2c183..d44c287 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -309,3 +309,134 @@ fn test_duplicate_classification_5_files() { ); } } + +#[test] +fn test_fingerprint_nonexistent_file_returns_open_failed() { + use dupsonic::fingerprint::FingerprintError; + + let path = PathBuf::from("/nonexistent/path/to/audio.flac"); + let result = fingerprint_file(&path, 120); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + matches!(err, FingerprintError::OpenFailed(_)), + "Expected OpenFailed, got: {err:?}" + ); + // Verify Display message is user-friendly + let msg = err.to_string(); + assert!( + msg.contains("could not open file"), + "Error message should be user-friendly, got: {msg}" + ); +} + +#[test] +fn test_fingerprint_non_audio_file_returns_unrecognized_format() { + use dupsonic::fingerprint::FingerprintError; + + // Create a temporary file with garbage content but an audio extension + let dir = TempDir::new().unwrap(); + let fake_audio = dir.path().join("not_audio.flac"); + std::fs::write(&fake_audio, b"This is not audio data at all").unwrap(); + + let result = fingerprint_file(&fake_audio, 120); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + matches!(err, FingerprintError::UnrecognizedFormat(_)), + "Expected UnrecognizedFormat, got: {err:?}" + ); + let msg = err.to_string(); + assert!( + msg.contains("not a recognized audio format"), + "Error message should be user-friendly, got: {msg}" + ); +} + +#[test] +fn test_fingerprint_truncated_file_returns_error() { + use dupsonic::fingerprint::FingerprintError; + + // Create a file with a valid FLAC magic number but truncated content + let dir = TempDir::new().unwrap(); + let truncated = dir.path().join("truncated.flac"); + // fLaC magic followed by garbage — enough to start probing but not enough to decode + let mut data = b"fLaC".to_vec(); + data.extend_from_slice(&[0u8; 100]); + std::fs::write(&truncated, &data).unwrap(); + + let result = fingerprint_file(&truncated, 120); + + assert!(result.is_err()); + let err = result.unwrap_err(); + // Could be UnrecognizedFormat or DecodeFailed depending on how far symphonia gets + assert!( + matches!( + err, + FingerprintError::UnrecognizedFormat(_) + | FingerprintError::DecodeFailed(_) + | FingerprintError::EmptyFingerprint + | FingerprintError::NoAudioTrack + ), + "Expected a structured error for truncated file, got: {err:?}" + ); + // Regardless of variant, Display should produce a useful message + let msg = err.to_string(); + assert!(!msg.is_empty(), "Error message should not be empty"); +} + +#[test] +fn test_fingerprint_empty_file_returns_unrecognized_format() { + use dupsonic::fingerprint::FingerprintError; + + let dir = TempDir::new().unwrap(); + let empty = dir.path().join("empty.mp3"); + std::fs::write(&empty, b"").unwrap(); + + let result = fingerprint_file(&empty, 120); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + matches!(err, FingerprintError::UnrecognizedFormat(_)), + "Expected UnrecognizedFormat for empty file, got: {err:?}" + ); +} + +#[test] +fn test_fingerprint_error_display_includes_details() { + use dupsonic::fingerprint::FingerprintError; + + // Verify that each error variant produces a non-empty, descriptive message + let errors = [ + FingerprintError::OpenFailed(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "permission denied", + )), + FingerprintError::UnrecognizedFormat("end of stream".to_string()), + FingerprintError::NoAudioTrack, + FingerprintError::DecodeFailed("unsupported codec".to_string()), + FingerprintError::EmptyFingerprint, + ]; + + let expected_fragments = [ + "could not open file", + "not a recognized audio format", + "no audio track found", + "unsupported or corrupted audio codec", + "could not extract audio content", + ]; + + for (err, expected) in errors.iter().zip(expected_fragments.iter()) { + let msg = err.to_string(); + assert!( + msg.contains(expected), + "FingerprintError::{:?} display should contain '{}', got: '{}'", + err, + expected, + msg + ); + } +}