fix(studio): surface a render panic instead of caching an empty frame

render_frame panics on failure (.expect("render frame"), .expect("rgba
matches dimensions"), .expect("encode jpeg")). Because the panic happens in
the scoped child thread and is consumed by join().unwrap_or_default(), the
catch_unwind wrapper in view.rs::serve_or_render never fires — so
fail_ledger().record_failure(key) is unreachable on that path and the whole
retry-budget mechanism is dead code for the on-demand asset handler. Instead
the empty Vec is treated as a successful render: frame_cache().insert(key,
rendered.clone(), ...) caches zero bytes, and the responder replies 200
image/jpeg with an empty body. The canvas shows a permanently broken image
for that (generation, frame, scale) and the cache guarantees it is never re-
attempted. (The prefetch worker path calls render_frame directly and is
correctly fenced.)

Fix: Return Result from render_frame_deep (propagate join()'s Err instead of
unwrap_or_default), or at minimum refuse to cache/serve an empty byte vector
and record it in the fail ledger so serve_or_render returns 500 and the
webview keeps the previous frame.

Refs #220

Author: LeadcodeDev

crates/rustmotion-studio/src/editor/frames.rs

@@ -26,20 +26,25 @@ pub const RENDER_STACK: usize = 32 * 1024 * 1024;
 
 /// [`render_frame`] on a thread with [`RENDER_STACK`], for callers that are not
 /// on the main thread. Scoped, so the scenario and tasks are borrowed rather
-/// than cloned.
+/// than cloned. `Err` means the render thread panicked (a Skia panic, a
+/// dimension mismatch, a JPEG encode failure): the caller must not treat that
+/// as a successful empty frame.
+// The panic payload carries no information callers act on; they only branch
+// on success vs. failure (retry-budget ledger, cache, HTTP status).
+#[allow(clippy::result_unit_err)]
 pub fn render_frame_deep(
     scenario: &ResolvedScenario,
     tasks: &[rustmotion::encode::video::FrameTask],
     frame: u32,
     scale: f32,
-) -> Vec<u8> {
+) -> Result<Vec<u8>, ()> {
     std::thread::scope(|scope| {
         std::thread::Builder::new()
             .stack_size(RENDER_STACK)
             .spawn_scoped(scope, || render_frame(scenario, tasks, frame, scale))
             .expect("spawn render thread")
             .join()
-            .unwrap_or_default()
+            .map_err(|_| ())
     })
 }
 

crates/rustmotion-studio/src/editor/view.rs

@@ -361,6 +361,10 @@ pub fn StudioApp(view: Signal<View>) -> Element {
 /// into the cache for the next request. `gen_b` is `Some(model generation)`
 /// when serving side B (drives stale-generation eviction); side A passes the
 /// baseline hash inside `key.generation` and `None` here.
+///
+/// `render_frame_deep` already isolates a render panic inside its own scoped
+/// thread and reports it as `Err`; the `catch_unwind` here only guards the
+/// (rarer) case of the OS failing to spawn that thread.
 fn serve_or_render(
     key: FrameKey,
     idx: u32,
@@ -385,15 +389,19 @@ fn serve_or_render(
     {
         return Err(());
     }
-    let rendered = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+    let outer = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
         render_frame_deep(scenario, tasks, idx, scale_factor(key.scale_pct))
-    }))
-    .map_err(|_| {
-        fail_ledger()
-            .lock()
-            .unwrap_or_else(|e| e.into_inner())
-            .record_failure(key);
-    })?;
+    }));
+    let rendered = match outer {
+        Ok(Ok(bytes)) => bytes,
+        _ => {
+            fail_ledger()
+                .lock()
+                .unwrap_or_else(|e| e.into_inner())
+                .record_failure(key);
+            return Err(());
+        }
+    };
     let (gen_b, gen_a) = match key.side {
         DiffSide::B => (gen_b, None),
         DiffSide::A => (None, Some(key.generation)),

crates/rustmotion-studio/src/lib.rs

@@ -1,6 +1,6 @@
 mod app;
 mod components;
-mod editor;
+pub mod editor;
 mod library;
 pub mod scenario;
 

crates/rustmotion-studio/src/library/data.rs

@@ -132,9 +132,7 @@ pub fn render_thumbnail(path: &Path) -> Option<Vec<u8>> {
     if tasks.is_empty() {
         return None;
     }
-    Some(crate::editor::frames::render_frame_deep(
-        &scenario, &tasks, 0, 0.25,
-    ))
+    crate::editor::frames::render_frame_deep(&scenario, &tasks, 0, 0.25).ok()
 }
 
 /// Cheap check: is this JSON a Rustmotion scenario? Avoids the heavier

crates/rustmotion-studio/tests/audit_ws_g.rs

@@ -1,23 +1,27 @@
-//! Regression tests for the studio's file-write pipeline: a debounced write
+//! Regression tests for the studio's file-write pipeline — a debounced write
 //! that must rebase onto the current disk instead of replaying a stale
 //! in-memory snapshot, coalesced edits inside one debounce window that must
 //! all survive (not just the last), and undo/redo cancelling a still-pending
-//! write before it can clobber the just-restored state.
+//! write before it can clobber the just-restored state — plus a render-thread
+//! panic that must surface as an error instead of being cached as an empty
+//! JPEG.
 //!
 //! `rustmotion-studio` has no `[dev-dependencies]` and cannot gain one in
 //! this change, so every test below drives the crate's existing public
-//! surface (`scenario::*`) against real temp files with plain synchronous
-//! `#[test]`s — no Dioxus runtime, no async executor. That public surface is
-//! itself the fix for the crate having no integration tests: the defects
-//! lived in a debounce timer and Dioxus event handlers that cannot be driven
-//! from a test, so each one was reduced to a pure decision over plain data
-//! (`resolve_flush`, the pending-write queue) and the handler calls that
-//! instead of deciding inline.
+//! surface (`scenario::*`, `editor::frames`) against real temp files with
+//! plain synchronous `#[test]`s — no Dioxus runtime, no async executor. That
+//! public surface is itself the fix for the crate having no integration
+//! tests: the defects lived in a debounce timer and Dioxus event handlers
+//! that cannot be driven from a test, so each one was reduced to a pure
+//! decision over plain data (`resolve_flush`, the pending-write queue,
+//! `render_frame_deep` returning `Result`) and the handler calls that instead
+//! of deciding inline.
 
 use std::fs;
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 
+use rustmotion_studio::editor::frames::render_frame_deep;
 use rustmotion_studio::scenario::{
     apply_optimistic, empty_scenario, pending_write_slot, queue_mutation, record_edit,
     resolve_flush, take_pending, undo, Mutation, Shared, SharedHistory, StudioModel,
@@ -219,3 +223,46 @@ fn the_write_pipeline_round_trips_an_edit_through_a_real_file() {
     );
     let _ = fs::remove_file(&path);
 }
+
+// ── A render-thread panic surfaces as an error, never a cached empty JPEG ──
+
+#[test]
+fn a_render_thread_panic_is_reported_as_an_error_not_an_empty_jpeg() {
+    let big = rustmotion::loader::load_scenario_from_source(
+        None,
+        Some(r##"{ "video": { "width": 64, "height": 64 }, "scenes": [ { "duration": 0.1 }, { "duration": 0.1 } ] }"##),
+    )
+    .unwrap();
+    let tasks = rustmotion::encode::build_frame_tasks(&big);
+    assert!(
+        tasks.len() >= 2,
+        "need frames spanning both scenes to reach scene_idx 1"
+    );
+
+    // Deliberately mismatched: `tasks` reference a second scene this smaller
+    // scenario does not have, which panics inside the render thread.
+    let small = rustmotion::loader::load_scenario_from_source(
+        None,
+        Some(r##"{ "video": { "width": 64, "height": 64 }, "scenes": [ { "duration": 0.1 } ] }"##),
+    )
+    .unwrap();
+
+    let last_frame = (tasks.len() - 1) as u32;
+    let result = render_frame_deep(&small, &tasks, last_frame, 1.0);
+    assert!(
+        result.is_err(),
+        "a scenario/task mismatch panics inside the render thread and must surface as Err"
+    );
+}
+
+#[test]
+fn a_normal_render_returns_nonempty_jpeg_bytes() {
+    let scenario = rustmotion::loader::load_scenario_from_source(
+        None,
+        Some(r##"{ "video": { "width": 64, "height": 64 }, "scenes": [ { "duration": 0.1 } ] }"##),
+    )
+    .unwrap();
+    let tasks = rustmotion::encode::build_frame_tasks(&scenario);
+    let jpeg = render_frame_deep(&scenario, &tasks, 0, 1.0).expect("a normal render succeeds");
+    assert!(!jpeg.is_empty());
+}