From 81479d2f647d343419bfa9791ab2da29f6acbcd2 Mon Sep 17 00:00:00 2001
From: h7x4 <h7x4@nani.wtf>
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<Value>,
+ 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..83087f9 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<Option<Value>, MpvError> {
+fn parse_mpv_response_data(value: Value, command: &[Value]) -> Result<Option<Value>, 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<Option<Value>, 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::<f64>("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::<f64>("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..99e9b8a 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::<bool>("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),
}