From 0e4b03ada0ebf52658b156880fc8dd392052cea2 Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@igalia.com>
Date: Fri, 9 Jun 2023 07:10:46 -0400
Subject: [PATCH] validate: Make use of rust more complex error types in action
 types

For sync action failures, instead of requiring users to report the error
through the Reporter interface, let it pass the detail of the failure into
the ActionError::Error directly and report it in the validate reporting
system automatically.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/1265>
---
 gstreamer-validate/src/action_type.rs | 30 ++++++++++++++++++++++-----
 gstreamer-validate/src/enums.rs       | 28 ++++++++++++++++---------
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/gstreamer-validate/src/action_type.rs b/gstreamer-validate/src/action_type.rs
index 391b48be5..db6e520a2 100644
--- a/gstreamer-validate/src/action_type.rs
+++ b/gstreamer-validate/src/action_type.rs
@@ -1,7 +1,7 @@
 use std::{ffi::c_int, ptr};
 
 use crate::ffi;
-use glib::translate::*;
+use glib::{object::Cast, translate::*};
 
 #[derive(Debug)]
 #[repr(transparent)]
@@ -325,14 +325,29 @@ impl<'a> ActionTypeBuilder<'a> {
         ) -> c_int {
             let action_type = ffi::gst_validate_get_action_type((*action).type_);
             let scenario = from_glib_borrow(scenario);
-            let action = crate::Action::from_glib_borrow(action);
+            let raction = from_glib_borrow(action);
 
             let func: &ActionFunction = &*(gst::ffi::gst_mini_object_get_qdata(
                 action_type as *mut gst::ffi::GstMiniObject,
                 QUARK_ACTION_TYPE_FUNC.get().unwrap().into_glib(),
             ) as *const Box<ActionFunction>);
 
-            (*func)(&scenario, action).into_glib()
+            let res = (*func)(&scenario, raction);
+
+            if let Err(crate::ActionError::Error(ref err)) = res {
+                scenario
+                    .dynamic_cast_ref::<crate::Reporter>()
+                    .unwrap()
+                    .report_action(
+                        &crate::Action::from_glib_none(action),
+                        glib::Quark::from_str("scenario::execution-error"),
+                        err,
+                    );
+
+                return ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED;
+            }
+
+            res.into_glib()
         }
 
         unsafe {
@@ -391,6 +406,7 @@ impl ActionTypeExtManual for crate::ActionType {
 #[cfg(test)]
 mod tests {
     use std::{
+        env,
         io::Write,
         sync::{Arc, Condvar, Mutex},
     };
@@ -399,6 +415,8 @@ mod tests {
     fn test_action_types() {
         gst::init().unwrap();
         use crate::prelude::*;
+        // Validate should not exit process on criticals
+        env::set_var("GST_VALIDATE", "");
         crate::init();
 
         let fails_called = Arc::new(Mutex::new(false));
@@ -410,7 +428,9 @@ mod tests {
                 move |_, _action| {
                     *fails_called.lock().unwrap() = true;
 
-                    Err(crate::ActionError::Error)
+                    Err(crate::ActionError::Error(
+                        "the `fails` action seems to fail".into(),
+                    ))
                 }
             ),
         )
@@ -509,7 +529,7 @@ mod tests {
         scenario.connect_action_done(glib::clone!(
             #[strong]
             async_called,
-            move |_, action| {
+            move |_, _| {
                 async_called.1.notify_one();
             }
         ));
diff --git a/gstreamer-validate/src/enums.rs b/gstreamer-validate/src/enums.rs
index 2a6d96eb5..0a5d11ff7 100644
--- a/gstreamer-validate/src/enums.rs
+++ b/gstreamer-validate/src/enums.rs
@@ -35,11 +35,10 @@ impl IntoGlib for ActionSuccess {
     }
 }
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 #[repr(i32)]
 pub enum ActionError {
-    Error = ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR,
-    ErrorReported = ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED,
+    Error(String),
     None = ffi::GST_VALIDATE_EXECUTE_ACTION_NONE,
 }
 
@@ -47,10 +46,11 @@ impl ActionError {
     pub fn from_value(value: impl Into<i32>) -> Self {
         skip_assert_initialized!();
         match value.into() {
-            ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR => ActionError::Error,
-            ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED => ActionError::ErrorReported,
+            ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR => {
+                ActionError::Error("Execution failed".to_string())
+            }
             ffi::GST_VALIDATE_EXECUTE_ACTION_NONE => ActionError::None,
-            _ => ActionError::Error,
+            _ => ActionError::Error("Unknown error".to_string()),
         }
     }
 }
@@ -60,7 +60,10 @@ impl IntoGlib for ActionError {
 
     #[inline]
     fn into_glib(self) -> ffi::GstValidateActionReturn {
-        self as ffi::GstValidateActionReturn
+        match self {
+            ActionError::Error(_) => ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR,
+            ActionError::None => ffi::GST_VALIDATE_EXECUTE_ACTION_NONE,
+        }
     }
 }
 
@@ -129,9 +132,10 @@ impl ActionReturn {
     #[inline]
     pub fn into_result(self) -> Result<ActionSuccess, ActionError> {
         match self {
-            Self::Error | Self::ErrorReported | Self::None => {
-                Err(unsafe { std::mem::transmute::<ActionReturn, ActionError>(self) })
+            Self::Error | Self::ErrorReported => {
+                Err(ActionError::Error("Execution failed".to_string()))
             }
+            Self::None => Err(ActionError::None),
             _ => Ok(unsafe { std::mem::transmute::<ActionReturn, ActionSuccess>(self) }),
         }
     }
@@ -139,7 +143,11 @@ impl ActionReturn {
     #[inline]
     pub fn from_error(v: ActionError) -> Self {
         skip_assert_initialized!();
-        unsafe { std::mem::transmute::<ActionError, ActionReturn>(v) }
+
+        match v {
+            ActionError::Error(_) => Self::Error,
+            ActionError::None => Self::None,
+        }
     }
 
     #[inline]