fix: error handling on selector

crates/composition/src/lib.rs

@@ -43,7 +43,7 @@ pub use layer_surface::{LayerSurfaceHandle, ShellSurfaceConfigHandler};
 pub use lock_selection::LockSelection;
 pub use popup::PopupShell;
 pub use popup_builder::PopupBuilder;
-pub use selection::Selection;
+pub use selection::{PropertyError, Selection, SelectionResult};
 pub use selector::{Output, Selector, Surface, SurfaceInfo};
 pub use session_lock::{SessionLock, SessionLockBuilder};
 pub use shell_runtime::{DEFAULT_SURFACE_NAME, ShellRuntime};
@@ -93,11 +93,12 @@ pub mod prelude {
         DEFAULT_SURFACE_NAME, EventDispatchContext, EventLoopHandle, Handle, IntoValue,
         KeyboardInteractivity, Layer, LayerSurfaceHandle, LockSelection, Output, OutputGeometry,
         OutputHandle, OutputInfo, OutputPolicy, OutputRegistry, PopupBuilder, PopupConfig,
-        PopupHandle, PopupPosition, PopupShell, PopupSize, PopupWindow, Result, Selection,
+        PopupHandle, PopupPosition, PopupShell, PopupSize, PopupWindow, PropertyError, Result,
-        Selector, SessionLock, SessionLockBuilder, Shell, ShellBuilder, ShellConfig, ShellControl,
+        Selection, SelectionResult, Selector, SessionLock, SessionLockBuilder, Shell, ShellBuilder,
-        ShellEventContext, ShellEventLoop, ShellRuntime, ShellSurfaceConfigHandler, Surface,
+        ShellConfig, ShellControl, ShellEventContext, ShellEventLoop, ShellRuntime,
-        SurfaceComponentConfig, SurfaceConfigBuilder, SurfaceControlHandle, SurfaceDefinition,
+        ShellSurfaceConfigHandler, Surface, SurfaceComponentConfig, SurfaceConfigBuilder,
-        SurfaceEntry, SurfaceHandle, SurfaceInfo, SurfaceMetadata, SurfaceRegistry,
+        SurfaceControlHandle, SurfaceDefinition, SurfaceEntry, SurfaceHandle, SurfaceInfo,
+        SurfaceMetadata, SurfaceRegistry,
     };
 
     pub use crate::calloop::{Generic, Interest, Mode, PostAction, RegistrationToken, Timer};

crates/composition/src/selection.rs

@@ -3,7 +3,79 @@ use crate::{
     selector::{Selector, SurfaceInfo},
     slint_interpreter::{ComponentInstance, Value},
 };
-use layer_shika_domain::errors::DomainError;
+use layer_shika_domain::{
+    errors::DomainError, value_objects::surface_instance_id::SurfaceInstanceId,
+};
+
+/// Result of a property operation on a single surface
+#[derive(Debug)]
+pub struct PropertyError {
+    pub surface_name: String,
+    pub instance_id: SurfaceInstanceId,
+    pub error: String,
+}
+
+/// Result of an operation on multiple selected surfaces
+#[derive(Debug)]
+pub struct SelectionResult<T> {
+    pub success_count: usize,
+    pub values: Vec<T>,
+    pub failures: Vec<PropertyError>,
+}
+
+impl<T> SelectionResult<T> {
+    fn new() -> Self {
+        Self {
+            success_count: 0,
+            values: Vec::new(),
+            failures: Vec::new(),
+        }
+    }
+
+    fn add_success(&mut self, value: T) {
+        self.success_count += 1;
+        self.values.push(value);
+    }
+
+    fn add_failure(&mut self, surface_name: String, instance_id: SurfaceInstanceId, error: String) {
+        self.failures.push(PropertyError {
+            surface_name,
+            instance_id,
+            error,
+        });
+    }
+
+    pub fn is_ok(&self) -> bool {
+        self.failures.is_empty()
+    }
+
+    pub fn is_partial_failure(&self) -> bool {
+        !self.failures.is_empty() && self.success_count > 0
+    }
+
+    pub fn is_total_failure(&self) -> bool {
+        !self.failures.is_empty() && self.success_count == 0
+    }
+
+    pub fn into_result(self) -> Result<Vec<T>, Error> {
+        if self.failures.is_empty() {
+            Ok(self.values)
+        } else {
+            let error_messages: Vec<String> = self
+                .failures
+                .iter()
+                .map(|e| format!("{}[{:?}]: {}", e.surface_name, e.instance_id, e.error))
+                .collect();
+            Err(Error::Domain(DomainError::Configuration {
+                message: format!(
+                    "Operation failed on {} surface(s): {}",
+                    self.failures.len(),
+                    error_messages.join(", ")
+                ),
+            }))
+        }
+    }
+}
 
 /// A selection of surfaces matching a selector
 ///
@@ -54,35 +126,55 @@ impl<'a> Selection<'a> {
     }
 
     /// Sets a property value on all matching surfaces
-    pub fn set_property(&self, name: &str, value: &Value) -> Result<(), Error> {
+    ///
-        let mut result = Ok(());
+    /// Returns a `SelectionResult` that contains information about both successes and failures.
-        self.shell.with_selected(&self.selector, |_, component| {
+    /// Use `.into_result()` to convert to a standard `Result` if you want fail-fast behavior,
-            if let Err(e) = component.set_property(name, value.clone()) {
+    /// or inspect `.failures` to handle partial failures gracefully.
-                log::error!("Failed to set property '{}': {}", name, e);
+    pub fn set_property(&self, name: &str, value: &Value) -> SelectionResult<()> {
-                result = Err(Error::Domain(DomainError::Configuration {
+        let mut result = SelectionResult::new();
-                    message: format!("Failed to set property '{}': {}", name, e),
+        self.shell
-                }));
+            .with_selected_info(&self.selector, |info, component| {
-            }
+                match component.set_property(name, value.clone()) {
-        });
+                    Ok(()) => result.add_success(()),
+                    Err(e) => {
+                        let error_msg = format!("Failed to set property '{}': {}", name, e);
+                        log::error!(
+                            "{} on surface {}[{:?}]",
+                            error_msg,
+                            info.name,
+                            info.instance_id
+                        );
+                        result.add_failure(info.name.clone(), info.instance_id, error_msg);
+                    }
+                }
+            });
         result
     }
 
     /// Gets property values from all matching surfaces
-    pub fn get_property(&self, name: &str) -> Result<Vec<Value>, Error> {
+    ///
-        let mut values = Vec::new();
+    /// Returns a `SelectionResult` containing all successfully retrieved values and any failures.
-        let mut result = Ok(());
+    /// Use `.into_result()` to convert to a standard `Result` if you want fail-fast behavior,
-        self.shell.with_selected(&self.selector, |_, component| {
+    /// or inspect `.values` and `.failures` to handle partial failures gracefully.
-            match component.get_property(name) {
+    pub fn get_property(&self, name: &str) -> SelectionResult<Value> {
-                Ok(value) => values.push(value),
+        let mut result = SelectionResult::new();
-                Err(e) => {
+        self.shell
-                    log::error!("Failed to get property '{}': {}", name, e);
+            .with_selected_info(&self.selector, |info, component| {
-                    result = Err(Error::Domain(DomainError::Configuration {
+                match component.get_property(name) {
-                        message: format!("Failed to get property '{}': {}", name, e),
+                    Ok(value) => result.add_success(value),
-                    }));
+                    Err(e) => {
+                        let error_msg = format!("Failed to get property '{}': {}", name, e);
+                        log::error!(
+                            "{} on surface {}[{:?}]",
+                            error_msg,
+                            info.name,
+                            info.instance_id
+                        );
+                        result.add_failure(info.name.clone(), info.instance_id, error_msg);
+                    }
                 }
-            }
+            });
-        });
+        result
-        result.map(|()| values)
     }
 
     /// Executes a configuration function with component and surface handle for matching surfaces

crates/composition/src/selector.rs

@@ -1,4 +1,5 @@
 use crate::{OutputHandle, OutputInfo};
+use layer_shika_domain::value_objects::surface_instance_id::SurfaceInstanceId;
 use std::fmt::{Debug, Formatter, Result as FmtResult};
 use std::sync::Arc;
 
@@ -9,6 +10,8 @@ pub struct SurfaceInfo {
     pub name: String,
     /// Handle to the output displaying this surface
     pub output: OutputHandle,
+    /// Unique identifier for this surface instance
+    pub instance_id: SurfaceInstanceId,
 }
 
 /// Selector for targeting surfaces when setting up callbacks or runtime configuration

crates/composition/src/shell.rs

@@ -1110,6 +1110,7 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: surface_name.clone(),
                 output: output_handle,
+                instance_id: SurfaceInstanceId::new(surface_handle, output_handle),
             };
 
             let output_info = system.app_state().get_output_info(output_handle);
@@ -1170,6 +1171,7 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: surface_name.clone(),
                 output: output_handle,
+                instance_id: SurfaceInstanceId::new(surface_handle, output_handle),
             };
 
             let output_info = system.app_state().get_output_info(output_handle);
@@ -1219,6 +1221,7 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: surface_name.to_string(),
                 output: key.output_handle,
+                instance_id: crate::SurfaceInstanceId::new(key.surface_handle, key.output_handle),
             };
 
             let output_info = system.app_state().get_output_info(key.output_handle);
@@ -1229,6 +1232,32 @@ impl Shell {
         }
     }
 
+    pub(crate) fn with_selected_info<F>(&self, selector: &crate::Selector, mut f: F)
+    where
+        F: FnMut(&crate::SurfaceInfo, &ComponentInstance),
+    {
+        let system = self.inner.borrow();
+        let (primary, active) = self.get_output_handles();
+
+        for (key, surface) in system.app_state().surfaces_with_keys() {
+            let surface_name = system
+                .app_state()
+                .get_surface_name(key.surface_handle)
+                .unwrap_or("unknown");
+            let surface_info = crate::SurfaceInfo {
+                name: surface_name.to_string(),
+                output: key.output_handle,
+                instance_id: crate::SurfaceInstanceId::new(key.surface_handle, key.output_handle),
+            };
+
+            let output_info = system.app_state().get_output_info(key.output_handle);
+
+            if selector.matches(&surface_info, output_info, primary, active) {
+                f(&surface_info, surface.component_instance());
+            }
+        }
+    }
+
     pub(crate) fn configure_selected<F>(&self, selector: &crate::Selector, mut f: F)
     where
         F: FnMut(&ComponentInstance, LayerSurfaceHandle<'_>),
@@ -1244,6 +1273,7 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: surface_name.to_string(),
                 output: key.output_handle,
+                instance_id: crate::SurfaceInstanceId::new(key.surface_handle, key.output_handle),
             };
 
             let output_info = system.app_state().get_output_info(key.output_handle);
@@ -1270,6 +1300,10 @@ impl Shell {
                 let surface_info = crate::SurfaceInfo {
                     name: surface_name.to_string(),
                     output: key.output_handle,
+                    instance_id: crate::SurfaceInstanceId::new(
+                        key.surface_handle,
+                        key.output_handle,
+                    ),
                 };
 
                 let output_info = system.app_state().get_output_info(key.output_handle);
@@ -1294,6 +1328,10 @@ impl Shell {
                 let surface_info = crate::SurfaceInfo {
                     name: surface_name.to_string(),
                     output: key.output_handle,
+                    instance_id: crate::SurfaceInstanceId::new(
+                        key.surface_handle,
+                        key.output_handle,
+                    ),
                 };
 
                 let output_info = system.app_state().get_output_info(key.output_handle);
@@ -1324,6 +1362,10 @@ impl Shell {
                 let surface_info = crate::SurfaceInfo {
                     name: component_name.to_string(),
                     output: output_handle,
+                    instance_id: crate::SurfaceInstanceId::new(
+                        crate::SurfaceHandle::from_raw(0),
+                        output_handle,
+                    ),
                 };
 
                 selector.matches(&surface_info, output_info, primary_handle, active_handle)
@@ -1423,6 +1465,10 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: component_name.clone(),
                 output: output_handle,
+                instance_id: crate::SurfaceInstanceId::new(
+                    crate::SurfaceHandle::from_raw(0),
+                    output_handle,
+                ),
             };
 
             let output_info = system.app_state().get_output_info(output_handle);
@@ -1446,6 +1492,10 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: component_name.clone(),
                 output: output_handle,
+                instance_id: crate::SurfaceInstanceId::new(
+                    crate::SurfaceHandle::from_raw(0),
+                    output_handle,
+                ),
             };
 
             let output_info = system.app_state().get_output_info(output_handle);
@@ -1473,6 +1523,10 @@ impl Shell {
             let surface_info = crate::SurfaceInfo {
                 name: component_name.clone(),
                 output: output_handle,
+                instance_id: crate::SurfaceInstanceId::new(
+                    crate::SurfaceHandle::from_raw(0),
+                    output_handle,
+                ),
             };
 
             let output_info = system.app_state().get_output_info(output_handle);