feat(signals): add WritableResultExt trait (#5338)

* feat(signals): add WritableResultExt trait

Add WritableResultExt mirroring the existing ReadableResultExt,
providing mutable access to signal values holding Result<T, E>.

Methods:
- as_mut(): returns Result<WritableRef<T>, WritableRef<E>>
- unwrap_mut(): returns WritableRef<T>, panics if Err

Closes #5293

* refactor(signals): use descriptive expect messages in WritableResultExt::as_mut

Replace bare .unwrap() calls with .expect() containing a descriptive
message about the failure mode. This gives better diagnostics if the
Result variant changes between the read and write lock acquisitions.

* switch to a single write to avoid race conditions with sync signals

---------

Co-authored-by: Evan Almloff <evanalmloff@gmail.com>

packages/signals/src/read.rs

@@ -419,9 +419,19 @@ pub trait ReadableResultExt<T, E>: Readable<Target = Result<T, E>> {
         T: 'static,
         E: 'static,
     {
-        <Self::Storage as AnyStorage>::try_map(self.read(), |v| v.as_ref().ok()).ok_or(
+        let read = self.read();
-            <Self::Storage as AnyStorage>::map(self.read(), |v| v.as_ref().err().unwrap()),
+        match read.deref() {
-        )
+            Ok(_) => Ok(<Self::Storage as AnyStorage>::map(read, |v| {
+                v.as_ref()
+                    .ok()
+                    .expect("Result variant changed between read and map")
+            })),
+            Err(_) => Err(<Self::Storage as AnyStorage>::map(read, |v| {
+                v.as_ref()
+                    .err()
+                    .expect("Result variant changed between read and map")
+            })),
+        }
     }
 }
 

packages/signals/src/write.rs

@@ -447,6 +447,44 @@ pub trait WritableOptionExt<T>: Writable<Target = Option<T>> {
 
 impl<T, W> WritableOptionExt<T> for W where W: Writable<Target = Option<T>> {}
 
+/// An extension trait for [`Writable<Result<T, E>>`] that provides some convenience methods.
+pub trait WritableResultExt<T, E>: Writable<Target = Result<T, E>> {
+    /// Unwraps the inner value mutably, panicking if the Result is an error.
+    #[track_caller]
+    fn unwrap_mut(&mut self) -> WritableRef<'_, Self, T>
+    where
+        T: 'static,
+        E: 'static,
+    {
+        WriteLock::filter_map(self.write(), |v| v.as_mut().ok())
+            .expect("Tried to unwrap a Result that was an error")
+    }
+
+    /// Attempts to mutably access the inner value of the Result.
+    #[track_caller]
+    fn as_mut(&mut self) -> Result<WritableRef<'_, Self, T>, WritableRef<'_, Self, E>>
+    where
+        T: 'static,
+        E: 'static,
+    {
+        let write = self.write();
+        match write.as_ref() {
+            Ok(_) => Ok(WriteLock::map(write, |v| {
+                v.as_mut()
+                    .ok()
+                    .expect("Result variant changed between read and write")
+            })),
+            Err(_) => Err(WriteLock::map(write, |v| {
+                v.as_mut()
+                    .err()
+                    .expect("Result variant changed between read and write")
+            })),
+        }
+    }
+}
+
+impl<T, E, W> WritableResultExt<T, E> for W where W: Writable<Target = Result<T, E>> {}
+
 /// An extension trait for [`Writable<Vec<T>>`] that provides some convenience methods.
 pub trait WritableVecExt<T>: Writable<Target = Vec<T>> {
     /// Pushes a new value to the end of the vector.