From 5e170a47c0793adfcccad896fba661b35002b49b Mon Sep 17 00:00:00 2001 From: h7x4 Date: Thu, 12 Dec 2024 16:26:44 +0100 Subject: [PATCH] MpvError: add copy of command for better context --- src/error.rs | 18 ++++++++++++++--- src/ipc.rs | 9 ++++++--- tests/integration_tests/misc.rs | 11 +++++++---- tests/mock_socket_tests/get_property.rs | 4 ++-- tests/mock_socket_tests/set_property.rs | 26 +++++++++++++------------ 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/error.rs b/src/error.rs index 9f4a860..16cfeba 100644 --- a/src/error.rs +++ b/src/error.rs @@ -8,8 +8,11 @@ use crate::{MpvDataType, Property}; /// Any error that can occur when interacting with mpv. #[derive(Error, Debug)] pub enum MpvError { - #[error("MpvError: {0}")] - MpvError(String), + #[error("Mpv returned error in response to command: {message}\nCommand: {command:#?}")] + MpvError { + command: Vec, + message: String, + }, #[error("Error communicating over mpv socket: {0}")] MpvSocketConnectionError(String), @@ -53,7 +56,16 @@ pub enum MpvError { impl PartialEq for MpvError { fn eq(&self, other: &Self) -> bool { match (self, other) { - (Self::MpvError(l0), Self::MpvError(r0)) => l0 == r0, + ( + Self::MpvError { + command: l_command, + message: l_message, + }, + Self::MpvError { + command: r_command, + message: r_message, + }, + ) => l_command == r_command && l_message == r_message, (Self::MpvSocketConnectionError(l0), Self::MpvSocketConnectionError(r0)) => l0 == r0, (Self::InternalConnectionError(l0), Self::InternalConnectionError(r0)) => l0 == r0, (Self::JsonParseError(l0), Self::JsonParseError(r0)) => { diff --git a/src/ipc.rs b/src/ipc.rs index bdb593b..c44eac7 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -92,7 +92,7 @@ impl MpvIpc { log::trace!("Received response: {:?}", response); - parse_mpv_response_data(response?) + parse_mpv_response_data(response?, command) } pub(crate) async fn get_mpv_property( @@ -197,7 +197,7 @@ impl MpvIpc { /// This function does the most basic JSON parsing and error handling /// for status codes and errors that all responses from mpv are /// expected to contain. -fn parse_mpv_response_data(value: Value) -> Result, MpvError> { +fn parse_mpv_response_data(value: Value, command: &[Value]) -> Result, MpvError> { log::trace!("Parsing mpv response data: {:?}", value); let result = value .as_object() @@ -225,7 +225,10 @@ fn parse_mpv_response_data(value: Value) -> Result, MpvError> { .and_then(|(error, data)| match error { "success" => Ok(data), "property unavailable" => Ok(None), - err => Err(MpvError::MpvError(err.to_string())), + err => Err(MpvError::MpvError { + command: command.to_owned(), + message: err.to_string(), + }) }); match &result { diff --git a/tests/integration_tests/misc.rs b/tests/integration_tests/misc.rs index 1d51948..2f73b71 100644 --- a/tests/integration_tests/misc.rs +++ b/tests/integration_tests/misc.rs @@ -47,10 +47,13 @@ async fn test_get_unavailable_property() -> Result<(), MpvError> { async fn test_get_nonexistent_property() -> Result<(), MpvError> { let (mut proc, mpv) = spawn_headless_mpv().await.unwrap(); let nonexistent = mpv.get_property::("nonexistent").await; - assert_eq!( - nonexistent, - Err(MpvError::MpvError("property not found".to_string())) - ); + + match nonexistent { + Err(MpvError::MpvError { message, .. }) => { + assert_eq!(message, "property not found"); + } + _ => panic!("Unexpected result: {:?}", nonexistent), + } mpv.kill().await.unwrap(); proc.kill().await.unwrap(); diff --git a/tests/mock_socket_tests/get_property.rs b/tests/mock_socket_tests/get_property.rs index 8639907..4b730ce 100644 --- a/tests/mock_socket_tests/get_property.rs +++ b/tests/mock_socket_tests/get_property.rs @@ -151,8 +151,8 @@ async fn test_get_property_simultaneous_requests() { tokio::time::sleep(Duration::from_millis(2)).await; let maybe_volume = mpv_clone_3.get_property::("nonexistent").await; match maybe_volume { - Err(MpvError::MpvError(err)) => { - assert_eq!(err, "property not found"); + Err(MpvError::MpvError { message, .. }) => { + assert_eq!(message, "property not found"); } _ => panic!("Unexpected result: {:?}", maybe_volume), } diff --git a/tests/mock_socket_tests/set_property.rs b/tests/mock_socket_tests/set_property.rs index 75ba0b2..67ebcc0 100644 --- a/tests/mock_socket_tests/set_property.rs +++ b/tests/mock_socket_tests/set_property.rs @@ -63,12 +63,12 @@ async fn test_set_property_wrong_type() -> Result<(), MpvError> { let mpv = Mpv::connect_socket(server).await?; let maybe_volume = mpv.set_property::("volume", true).await; - assert_eq!( - maybe_volume, - Err(MpvError::MpvError( - "unsupported format for accessing property".to_string() - )) - ); + match maybe_volume { + Err(MpvError::MpvError { message, .. }) => { + assert_eq!(message, "unsupported format for accessing property"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); @@ -84,10 +84,12 @@ async fn test_get_property_error() -> Result<(), MpvError> { let mpv = Mpv::connect_socket(server).await?; let maybe_volume = mpv.set_property("nonexistent", true).await; - assert_eq!( - maybe_volume, - Err(MpvError::MpvError("property not found".to_string())) - ); + match maybe_volume { + Err(MpvError::MpvError { message, .. }) => { + assert_eq!(message, "property not found"); + } + _ => panic!("Unexpected result: {:?}", maybe_volume), + } join_handle.await.unwrap().unwrap(); @@ -161,8 +163,8 @@ async fn test_set_property_simultaneous_requests() { tokio::time::sleep(Duration::from_millis(2)).await; let maybe_volume = mpv_clone_3.set_property("nonexistent", "a").await; match maybe_volume { - Err(MpvError::MpvError(err)) => { - assert_eq!(err, "property not found"); + Err(MpvError::MpvError{ message, .. }) => { + assert_eq!(message, "property not found"); } _ => panic!("Unexpected result: {:?}", maybe_volume), }