blob-lease: improve error handling

If someone removed the directory structure, or otherwise messed with
the files, there was a good chance that we'd end up in a panic.

This commit improves the error messaging produced by the blob
lease layer to provide the path as context, log the error,
and then translate the error case to an empty frame so that
the rest of the rendering machinery can proceed in a more reasonable
way.

We take care to log this sort of corrupt frame error only once,
as it is most likely to occur in an animated gif and will thus
trigger multiple times per second for such a file.

refs: #5422

docs/changelog.md

@@ -82,7 +82,10 @@ As features stabilize some brief notes about them will accumulate here.
 * Search mode now accepts composed input from the IME. Thanks to @kenchou! #5564
 * Quick select mode will now accept unix paths with `//` in them. #5763
 * blob leases (for image rendering) could be removed by temporary directory
-  cleaners, resulting in issues with rendering. #5422
+  cleaners, resulting in issues with rendering. We no longer store these
+  in a pure temporary directory; they live in a cache dir, and if someone
+  does remove or truncate these files, we now convert that error case
+  into blank frame(s). #5422
 
 #### Updated
 * Bundled conpty.dll and OpenConsole.exe to build 1.19.240130002.nupkg

wezterm-blob-leases/src/error.rs

@@ -1,4 +1,5 @@
 use crate::ContentId;
+use std::path::PathBuf;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
@@ -17,4 +18,7 @@ pub enum Error {
 
     #[error("Storage has not been initialized")]
     StorageNotInit,
+
+    #[error("Storage location {0} may be corrupt: {1}")]
+    StorageDirIoError(PathBuf, std::io::Error),
 }

wezterm-blob-leases/src/simple_tempdir.rs

@@ -40,7 +40,8 @@ impl SimpleTempDir {
 
     fn path_for_content(&self, content_id: ContentId) -> Result<PathBuf, Error> {
         let path = self.root.path().join(format!("{content_id}"));
-        std::fs::create_dir_all(path.parent().unwrap())?;
+        std::fs::create_dir_all(path.parent().unwrap())
+            .map_err(|err| Error::StorageDirIoError(path.clone(), err))?;
         Ok(path)
     }
 
@@ -104,7 +105,7 @@ impl BlobStorage for SimpleTempDir {
         let _refs = self.refs.lock().unwrap();
 
         let path = self.path_for_content(content_id)?;
-        Ok(std::fs::read(&path)?)
+        Ok(std::fs::read(&path).map_err(|err| Error::StorageDirIoError(path, err))?)
     }
 
     fn get_reader(&self, content_id: ContentId, lease_id: LeaseId) -> Result<BoxedReader, Error> {

wezterm-gui/src/glyphcache.rs

@@ -20,6 +20,7 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 use std::io::Seek;
 use std::rc::Rc;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::mpsc::{sync_channel, Receiver, RecvTimeoutError, SyncSender, TryRecvError};
 use std::sync::{Arc, MutexGuard};
 use std::time::{Duration, Instant};
@@ -31,6 +32,20 @@ use wezterm_font::units::*;
 use wezterm_font::{FontConfiguration, GlyphInfo, LoadedFont, LoadedFontId};
 use wezterm_term::Underline;
 
+static FRAME_ERROR_REPORTED: AtomicBool = AtomicBool::new(false);
+
+/// We only want to report a frame error once at error level, because
+/// if it is triggering it is likely in a animated image and will continue
+/// to trigger multiple times per second as the frames are cycled.
+fn report_frame_error<S: Into<String>>(message: S) {
+    if FRAME_ERROR_REPORTED.load(Ordering::Relaxed) {
+        log::debug!("{}", message.into());
+    } else {
+        log::error!("{}", message.into());
+        FRAME_ERROR_REPORTED.store(true, Ordering::Relaxed);
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum LoadState {
     Loading,
@@ -1025,14 +1040,35 @@ impl GlyphCache {
                     return Ok((sprite.clone(), next, frames.load_state));
                 }
 
+                let expected_byte_size =
+                    frames.current_frame.width * frames.current_frame.height * 4;
+
+                let frame_data = match frames.current_frame.lease.get_data() {
+                    Ok(data) => {
+                        // If the size isn't right, ignore this frame and replace
+                        // it with a blank one instead. This might happen if
+                        // some process is truncating the files, or perhaps if
+                        // the disk is full.
+                        // We need to check for this because the consequence of
+                        // a mismatched size is a panic in a layer where we
+                        // cannot handle the error case.
+                        if data.len() != expected_byte_size {
+                            report_frame_error(format!("frame data is corrupted: expected size {expected_byte_size} but have {}", data.len()));
+                            vec![0u8; expected_byte_size]
+                        } else {
+                            data
+                        }
+                    }
+                    Err(err) => {
+                        report_frame_error(format!("frame data error: {err:#}"));
+                        vec![0u8; expected_byte_size]
+                    }
+                };
+
                 let frame = Image::from_raw(
                     frames.current_frame.width,
                     frames.current_frame.height,
-                    frames
-                        .current_frame
-                        .lease
-                        .get_data()
-                        .context("frames.current_frame.lease.get_data")?,
+                    frame_data,
                 );
                 let sprite = atlas.allocate_with_padding(&frame, padding, scale_down)?;