Shader packs: a program the shader compiler panics on no longer crashes the game

Photon v1.4's gbuffers programs make naga's SPIR-V writer panic
("Expression [89] is not cached!") while wgpu-hal's Vulkan backend makes
the render pipeline in PackPost::new. Nothing caught it, so the game
crashed. The pack stays picked in horizon-graphics.txt, so every later
world load crashed again until the file was edited by hand (#3525).

- crash::contain runs a closure under catch_unwind and returns its
  panic as an error. The panic hook logs such a panic with its stack
  and makes no crash report.
- pack_post::checked, which builds every pack program's pipeline under
  a validation error scope, contains a panic too. The pass is skipped
  with a note, as a validation error already skips it. The rest of the
  pack runs, and the device keeps working.
- Any other panic while loading a pack (translation, PackPost::new)
  turns the pack off. Nothing runs in its place, it is unpicked and
  saved so the next start does not load it again, and the chat says so
  in red, as a refused Horizon pack already does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/horizon-client-render/src/crash.rs

@@ -3,6 +3,7 @@
 //! Written to `crash-reports/crash-<yyyy-MM-dd_HH.mm.ss>-client.txt` in the game folder when
 //! the main thread panics or `main` gets an error back. It leaves the machine only scrubbed, and
 //! only if the player allows crash reports (`telemetry`).
+use std::cell::{Cell, RefCell};
 use std::fmt::Write as _;
 use std::path::{Path, PathBuf};
 use std::sync::Mutex;
@@ -14,6 +15,51 @@ use chrono::{DateTime, Local};
 static DETAILS: Mutex<Vec<(&'static str, String)>> = Mutex::new(Vec::new());
 static REPORTED: AtomicBool = AtomicBool::new(false);
 
+thread_local! {
+    /// How many `contain` calls this thread is inside.
+    static CONTAINED: Cell<u32> = const { Cell::new(0) };
+    /// The message of the last panic `contain` caught on this thread, from the hook.
+    static CAUGHT: RefCell<Option<String>> = const { RefCell::new(None) };
+}
+
+/// Runs `f`, and returns a panic in it as an error with the panic's message instead of letting
+/// it end the game. For work that is not the game's own and whose failure the caller recovers
+/// from, such as handing a shader pack's programs to the GPU driver's compiler: the panic goes
+/// to the log with its stack, and no crash report is made for it.
+pub fn contain<T>(f: impl FnOnce() -> T) -> Result<T, String> {
+    CONTAINED.with(|c| c.set(c.get() + 1));
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(f));
+    CONTAINED.with(|c| c.set(c.get() - 1));
+    result.map_err(|payload| {
+        CAUGHT
+            .with(|c| c.borrow_mut().take())
+            .unwrap_or_else(|| payload_text(&*payload))
+    })
+}
+
+/// For the hook: a panic inside `contain` is logged with its stack and kept for `contain` to
+/// return. False for any other panic.
+fn contained_panic(info: &std::panic::PanicHookInfo) -> bool {
+    if CONTAINED.with(Cell::get) == 0 {
+        return false;
+    }
+    let message = panic_message(info);
+    let stack = stack_lines(
+        &std::backtrace::Backtrace::force_capture().to_string(),
+        true,
+    );
+    log::error!(
+        "Recovered from an error: {message}\n{}",
+        stack
+            .lines()
+            .map(|l| format!("\tat {l}"))
+            .collect::<Vec<_>>()
+            .join("\n")
+    );
+    CAUGHT.with(|c| *c.borrow_mut() = Some(message));
+    true
+}
+
 /// Sets one line of the report's details (replacing the last value under `key`).
 pub fn set_detail(key: &'static str, value: impl Into<String>) {
     let mut details = DETAILS.lock().unwrap_or_else(|e| e.into_inner());
@@ -308,13 +354,16 @@ fn deliver(home: &Path, report: Report) -> Option<PathBuf> {
     }
 }
 
-fn panic_message(info: &std::panic::PanicHookInfo) -> String {
-    let payload = info.payload();
-    let text = payload
+fn payload_text(payload: &(dyn std::any::Any + Send)) -> String {
+    payload
         .downcast_ref::<&str>()
         .map(|s| s.to_string())
         .or_else(|| payload.downcast_ref::<String>().cloned())
-        .unwrap_or_else(|| "Box<dyn Any>".into());
+        .unwrap_or_else(|| "Box<dyn Any>".into())
+}
+
+fn panic_message(info: &std::panic::PanicHookInfo) -> String {
+    let text = payload_text(info.payload());
     match info.location() {
         Some(at) => format!("panicked at {at}:\n{text}"),
         None => format!("panicked:\n{text}"),
@@ -417,10 +466,13 @@ pub fn panic_report(info: &std::panic::PanicHookInfo, side: &'static str) -> Rep
 
 /// Reports panics: on the main thread a crash report (the panic then ends the game, exit code
 /// 101); on other threads the error goes to the log, as vanilla's uncaught exception handler
-/// does. The default hook still prints the panic.
+/// does. The default hook still prints the panic. A panic inside `contain` is only logged.
 pub fn install(home: PathBuf, side: &'static str) {
     let default = std::panic::take_hook();
     std::panic::set_hook(Box::new(move |info| {
+        if contained_panic(info) {
+            return;
+        }
         default(info);
         let main = std::thread::current().name() == Some("main");
         if main {
@@ -609,6 +661,17 @@ mod tests {
         );
     }
 
+    #[test]
+    fn contained_panics_come_back_as_errors() {
+        assert_eq!(contain(|| 7), Ok(7));
+        let error = contain(|| -> u8 { panic!("a pack's program") }).unwrap_err();
+        assert!(error.contains("a pack's program"), "{error}");
+        // Nested, and the thread is no longer inside one after.
+        let outer = contain(|| contain(|| -> u8 { panic!("inner") }));
+        assert!(outer.unwrap().unwrap_err().contains("inner"));
+        assert_eq!(CONTAINED.with(Cell::get), 0);
+    }
+
     #[test]
     fn the_debug_crash_is_reported_as_vanillas() {
         let error: Box<dyn std::error::Error> = DebugCrash::new().into();

crates/horizon-client-render/src/gfx/pack_post.rs

@@ -476,10 +476,12 @@ fn sampler(
     })
 }
 
-/// Runs `create` and returns its validation error, if any.
+/// Runs `create` and returns its validation error, if any. A panic while it runs is an error
+/// too: naga's backends turn a module they cannot write into a panic as the driver gets the
+/// pipeline (#3525), which a pack's program must not turn into a crash.
 fn checked<T>(device: &wgpu::Device, create: impl FnOnce() -> T) -> Result<T, String> {
     let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
-    let value = create();
+    let value = crate::crash::contain(create);
     match pollster::block_on(scope.pop()) {
         Some(e) => {
             // The details are in the error's description or its sources.
@@ -494,7 +496,7 @@ fn checked<T>(device: &wgpu::Device, create: impl FnOnce() -> T) -> Result<T, St
             }
             Err(message)
         }
-        None => Ok(value),
+        None => value.map_err(|panic| format!("the shader compiler failed: {panic}")),
     }
 }
 
@@ -2629,6 +2631,38 @@ mod tests {
         assert_eq!(used, [1, 2, 3]);
     }
 
+    /// #3525: naga's SPIR-V writer panicked on Photon's gbuffers programs as the pipeline was
+    /// made. Such a panic skips the pass, as a validation error does, and the device goes on.
+    #[test]
+    fn a_shader_compiler_panic_skips_the_pass() {
+        let instance = wgpu::Instance::new(wgpu::InstanceDescriptor::new_without_display_handle());
+        let Ok(adapter) = pollster::block_on(instance.request_adapter(&Default::default())) else {
+            return;
+        };
+        let Ok((device, _queue)) = pollster::block_on(adapter.request_device(&Default::default()))
+        else {
+            return;
+        };
+        let failed = checked(&device, || -> wgpu::Buffer {
+            panic!("internal error: entered unreachable code: Expression [89] is not cached!")
+        });
+        let error = failed.expect_err("the panic comes back as an error");
+        assert!(error.contains("Expression [89] is not cached!"), "{error}");
+        let buffer = |usage| {
+            checked(&device, || {
+                device.create_buffer(&wgpu::BufferDescriptor {
+                    label: None,
+                    size: 16,
+                    usage,
+                    mapped_at_creation: false,
+                })
+            })
+        };
+        // The device still makes things, and its errors still come back.
+        assert!(buffer(wgpu::BufferUsages::UNIFORM).is_ok());
+        assert!(buffer(wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::MAP_WRITE).is_err());
+    }
+
     #[test]
     fn pack_formats_map_to_renderable_ones() {
         let f = |name| texture_format(Format::parse(name).unwrap(), wgpu::Features::empty());

crates/horizon-client/src/game/shader_packs.rs

@@ -6,6 +6,7 @@
 
 use horizon_client::camera::CameraFluid;
 use horizon_client::gfx::pack_uniforms::PackInputs;
+use horizon_client::text::Text;
 use horizon_shaders::{BlockIds, Macros, Pack, Properties, dimension_folder, load_post};
 
 use super::{Dim, Game};
@@ -64,79 +65,87 @@ impl Game<'_> {
         {
             let name = name.as_str();
             let started = std::time::Instant::now();
-            match Pack::open(&path) {
-                Ok(pack) => {
-                    let macros = Macros::standard();
-                    let text = pack.read_text("shaders.properties").unwrap_or_default();
-                    let properties = Properties::parse(&text, &macros);
-                    for note in &properties.notes {
-                        log::warn!(
-                            "shader pack {name}: shaders.properties:{}: {}",
-                            note.line,
-                            note.message
-                        );
-                    }
-                    let folder = dimension_folder(&pack, dimension);
-                    let inputs = renderer.pack_inputs();
-                    let mut post = load_post(&pack, &folder, &properties, &macros, &inputs);
-                    let biomes = self.res.biomes();
-                    let names: Vec<&str> = biomes.list.iter().map(|b| b.name.as_str()).collect();
-                    post.custom.set_biomes(&names);
-                    let text = pack.read_text("block.properties").unwrap_or_default();
-                    let ids = BlockIds::parse(&Properties::parse(&text, &macros));
-                    for note in &ids.notes {
-                        log::warn!("shader pack {name}: block.properties: {note}");
-                    }
-                    let text = pack.read_text("entity.properties");
-                    let entities = BlockIds::parse_kind(
-                        &Properties::parse(text.as_deref().unwrap_or_default(), &macros),
-                        "entity",
-                    );
-                    let text = pack.read_text("item.properties");
-                    let items = BlockIds::parse_kind(
-                        &Properties::parse(text.as_deref().unwrap_or_default(), &macros),
-                        "item",
-                    );
-                    self.pack_ids = Some(PackIds {
-                        items: items.plain(),
-                        missing_item: if text.is_some() { -1 } else { 0 },
-                        // OQ-21: on unless the pack turns it off.
-                        old_hand_light: properties
-                            .get("oldHandLight")
-                            .is_none_or(|v| v.trim() != "false"),
-                        entities: entities.plain(),
-                        missing_entity: if text.is_some() { 65535 } else { 0 },
-                        missing_block: if ids.is_empty() { 0 } else { 65535 },
-                        blocks: ids.clone(),
-                        block_states: RefCell::default(),
-                    });
-                    let started_ids = std::time::Instant::now();
-                    let table = block_id_table(&self.pack, &ids);
-                    log::info!(
-                        "shader pack {name}: mc_Entity for {} of {} block sprites in {} ms",
-                        table.chunks(2).filter(|p| p[0] != -1).count(),
-                        table.len() / 2,
-                        started_ids.elapsed().as_millis()
-                    );
-                    if let Some((programs, notes)) =
-                        renderer.set_shader_pack(dimension, Some(post), &table)
-                    {
-                        for note in notes {
-                            log::warn!("shader pack {name}: {note}");
+            // A pack is not the game's own code: one that makes the shader compiler (or our
+            // translation of it) panic is turned off instead of crashing every world load (#3525).
+            let loaded = horizon_client::crash::contain(|| {
+                match Pack::open(&path) {
+                    Ok(pack) => {
+                        let macros = Macros::standard();
+                        let text = pack.read_text("shaders.properties").unwrap_or_default();
+                        let properties = Properties::parse(&text, &macros);
+                        for note in &properties.notes {
+                            log::warn!(
+                                "shader pack {name}: shaders.properties:{}: {}",
+                                note.line,
+                                note.message
+                            );
                         }
-                        log::info!(
-                            "shader pack {name}: folder `{folder}`, {} programs ({}) in {} ms",
-                            programs.len(),
-                            programs.join(" "),
-                            started.elapsed().as_millis()
+                        let folder = dimension_folder(&pack, dimension);
+                        let inputs = renderer.pack_inputs();
+                        let mut post = load_post(&pack, &folder, &properties, &macros, &inputs);
+                        let biomes = self.res.biomes();
+                        let names: Vec<&str> =
+                            biomes.list.iter().map(|b| b.name.as_str()).collect();
+                        post.custom.set_biomes(&names);
+                        let text = pack.read_text("block.properties").unwrap_or_default();
+                        let ids = BlockIds::parse(&Properties::parse(&text, &macros));
+                        for note in &ids.notes {
+                            log::warn!("shader pack {name}: block.properties: {note}");
+                        }
+                        let text = pack.read_text("entity.properties");
+                        let entities = BlockIds::parse_kind(
+                            &Properties::parse(text.as_deref().unwrap_or_default(), &macros),
+                            "entity",
                         );
+                        let text = pack.read_text("item.properties");
+                        let items = BlockIds::parse_kind(
+                            &Properties::parse(text.as_deref().unwrap_or_default(), &macros),
+                            "item",
+                        );
+                        self.pack_ids = Some(PackIds {
+                            items: items.plain(),
+                            missing_item: if text.is_some() { -1 } else { 0 },
+                            // OQ-21: on unless the pack turns it off.
+                            old_hand_light: properties
+                                .get("oldHandLight")
+                                .is_none_or(|v| v.trim() != "false"),
+                            entities: entities.plain(),
+                            missing_entity: if text.is_some() { 65535 } else { 0 },
+                            missing_block: if ids.is_empty() { 0 } else { 65535 },
+                            blocks: ids.clone(),
+                            block_states: RefCell::default(),
+                        });
+                        let started_ids = std::time::Instant::now();
+                        let table = block_id_table(&self.pack, &ids);
+                        log::info!(
+                            "shader pack {name}: mc_Entity for {} of {} block sprites in {} ms",
+                            table.chunks(2).filter(|p| p[0] != -1).count(),
+                            table.len() / 2,
+                            started_ids.elapsed().as_millis()
+                        );
+                        if let Some((programs, notes)) =
+                            renderer.set_shader_pack(dimension, Some(post), &table)
+                        {
+                            for note in notes {
+                                log::warn!("shader pack {name}: {note}");
+                            }
+                            log::info!(
+                                "shader pack {name}: folder `{folder}`, {} programs ({}) in {} ms",
+                                programs.len(),
+                                programs.join(" "),
+                                started.elapsed().as_millis()
+                            );
+                        }
+                    }
+                    Err(e) => {
+                        self.pack_ids = None;
+                        log::warn!("shader pack {name}: {e}");
+                        renderer.set_shader_pack(dimension, None, &[]);
                     }
                 }
-                Err(e) => {
-                    self.pack_ids = None;
-                    log::warn!("shader pack {name}: {e}");
-                    renderer.set_shader_pack(dimension, None, &[]);
-                }
+            });
+            if let Err(panic) = loaded {
+                self.shader_pack_failed(name, dimension, &panic);
             }
         }
         let lerp = |(old, new): (f32, f32)| old + (new - old) * partial;
@@ -175,6 +184,25 @@ impl Game<'_> {
         }
     }
 
+    /// Turns off the pack `name` after loading it panicked: nothing runs in its place, it is no
+    /// longer the picked pack (so the next start does not load it again), and the chat says so.
+    fn shader_pack_failed(&mut self, name: &str, dimension: i32, error: &str) {
+        log::error!("shader pack {name}: turned off: {error}");
+        self.pack_ids = None;
+        if let Some(renderer) = &mut self.renderer {
+            // Marks this dimension as loaded, without a pack, so it is not tried again.
+            renderer.set_shader_pack(dimension, None, &[]);
+        }
+        if self.settings.horizon.shader_pack == name {
+            self.settings.horizon.shader_pack.clear();
+            self.settings.save(&Self::options_path());
+        }
+        self.system_message(Text::colored(
+            format!("Shader pack {name} could not be loaded and was turned off"),
+            0xFF5555,
+        ));
+    }
+
     /// `heldItemId(2)` and `heldBlockLightValue(2)`: main hand, then off hand (5.2, 5.3).
     fn held_for_pack(&mut self) -> ([i32; 2], [i32; 2]) {
         if self.pack_ids.is_none() {