From 37a3ab4177aafaec3ade7ad5cddd91ab72032f79 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 23:08:54 +0000 Subject: [PATCH 1/2] #1507: web start-up treats only NotFoundError as a missing file OPFS lookups turned any exception into "absent", and start-up only logged other read errors, so a library with an unreadable file opened incomplete and the next save could overwrite the stored copy. Only NotFoundError now means absent; when a stored library file or the active-library pointer can't be read, the library isn't opened, nothing is saved and the window names the file. An unreadable thumbnail index no longer deletes every stored thumbnail as unindexed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- apps/lightcraft-web/src/backend.rs | 15 +++++--- apps/lightcraft-web/src/files.rs | 55 +++++++++++++++++++++++++++ apps/lightcraft-web/src/web.rs | 61 ++++++++++++++++++++---------- docs/web.md | 5 +++ docs/whats-new.md | 8 ++++ 5 files changed, 119 insertions(+), 25 deletions(-) diff --git a/apps/lightcraft-web/src/backend.rs b/apps/lightcraft-web/src/backend.rs index e20f6dc9c..1dbc2266b 100644 --- a/apps/lightcraft-web/src/backend.rs +++ b/apps/lightcraft-web/src/backend.rs @@ -22,6 +22,12 @@ fn err(e: JsValue) -> String { e.as_string().or_else(|| e.dyn_ref::().map(|e| String::from(e.message()))).unwrap_or_else(|| format!("{e:?}")) } +/// Did an OPFS call fail because the entry doesn't exist (`NotFoundError`)? Any other failure +/// (security, type mismatch, an unreadable or evicted store) is an error, never "absent" (#1507). +fn not_found(e: &JsValue) -> bool { + crate::files::is_not_found(Reflect::get(e, &"name".into()).ok().and_then(|n| n.as_string()).as_deref()) +} + /// `navigator.storage` in a window or a worker. fn storage_manager() -> Option { let g = js_sys::global(); @@ -134,10 +140,7 @@ impl Backend { opts.set_create(create); match JsFuture::from(dir.get_directory_handle_with_options(p, &opts)).await { Ok(d) => dir = d.unchecked_into(), - Err(e) if !create => { - let _ = e; - return Ok(None); - } + Err(e) if !create && not_found(&e) => return Ok(None), Err(e) => return Err(err(e)), } } @@ -150,7 +153,7 @@ impl Backend { opts.set_create(create); match JsFuture::from(dir.get_file_handle_with_options(&name, &opts)).await { Ok(h) => Ok(Some(h.unchecked_into())), - Err(_) if !create => Ok(None), + Err(e) if !create && not_found(&e) => Ok(None), Err(e) => Err(err(e)), } } @@ -250,7 +253,7 @@ impl Backend { let Some((dir, name)) = Self::parent(root, path, false).await? else { return Ok(()) }; match JsFuture::from(dir.remove_entry(&name)).await { Ok(_) => Ok(()), - Err(e) if Reflect::get(&e, &"name".into()).ok().and_then(|n| n.as_string()).as_deref() == Some("NotFoundError") => Ok(()), + Err(e) if not_found(&e) => Ok(()), Err(e) => Err(err(e)), } } diff --git a/apps/lightcraft-web/src/files.rs b/apps/lightcraft-web/src/files.rs index d0c9f348b..cbb872016 100644 --- a/apps/lightcraft-web/src/files.rs +++ b/apps/lightcraft-web/src/files.rs @@ -198,6 +198,27 @@ impl Store for FilesStore { /// Names of the library files read at start-up. pub const LIBRARY_FILES: [&str; 6] = ["catalog.snap", "catalog.log", "presets.json", "view.json", "prefs.json", "ui.json"]; +/// Is a browser storage error, by its `DOMException` name, "the entry doesn't exist"? Only +/// `NotFoundError` is; anything else (security, type mismatch, an unreadable store) is a failure. +pub fn is_not_found(dom_exception_name: Option<&str>) -> bool { + dom_exception_name == Some("NotFoundError") +} + +/// Preload the library files read at start-up (`None`: the file doesn't exist yet). A file that +/// exists but couldn't be read stops start-up instead (#1507): opening the library without it +/// would show an incomplete catalog and then save over the stored copy. +pub fn preload_library(files: &Files, reads: Vec<(&str, Result>, String>)>) -> Result<(), String> { + let mut failed = Vec::new(); + for (name, read) in reads { + match read { + Ok(Some(data)) => files.preload(name, data), + Ok(None) => {} + Err(e) => failed.push(format!("{name}: {e}")), + } + } + if failed.is_empty() { Ok(()) } else { Err(format!("couldn't read the library from browser storage ({})", failed.join("; "))) } +} + #[cfg(test)] mod tests { use super::*; @@ -282,6 +303,40 @@ mod tests { assert_eq!(s2.catalog.photo(id).unwrap().rating, 5, "the rating survived"); } + /// Start-up only treats a missing file as absent: a stored file that couldn't be read stops + /// it, naming the file, instead of opening an incomplete library (#1507). + #[test] + fn unreadable_library_file_stops_start_up() { + let files = Files::default(); + let reads = vec![ + ("catalog.snap", Ok(Some(b"snap".to_vec()))), + ("catalog.log", Err("NotReadableError: the file could not be read".to_string())), + ("presets.json", Ok(None)), + ]; + let e = preload_library(&files, reads).unwrap_err(); + assert!(e.contains("catalog.log") && e.contains("NotReadableError"), "{e}"); + } + + /// A new library (nothing stored yet) still starts. + #[test] + fn missing_library_files_are_a_new_library() { + let files = Files::default(); + let reads = LIBRARY_FILES.iter().map(|n| (*n, Ok(None))).collect(); + preload_library(&files, reads).unwrap(); + assert!(files.get("catalog.snap").is_none()); + let reads = vec![("catalog.snap", Ok(Some(b"snap".to_vec()))), ("catalog.log", Ok(None))]; + preload_library(&files, reads).unwrap(); + assert_eq!(files.get("catalog.snap").as_deref(), Some(&b"snap"[..])); + } + + #[test] + fn only_not_found_errors_mean_absent() { + assert!(is_not_found(Some("NotFoundError"))); + for name in [Some("SecurityError"), Some("TypeMismatchError"), Some("NotReadableError"), Some("InvalidStateError"), None] { + assert!(!is_not_found(name), "{name:?}"); + } + } + #[test] fn library_survives_reload_through_flushes() { let mut disk = BTreeMap::new(); diff --git a/apps/lightcraft-web/src/web.rs b/apps/lightcraft-web/src/web.rs index 6a4487d09..6ac04864f 100644 --- a/apps/lightcraft-web/src/web.rs +++ b/apps/lightcraft-web/src/web.rs @@ -280,6 +280,9 @@ struct Boot { index: ThumbIndex, /// The library folder in storage (`library`, or a restored backup's). lib_dir: String, + /// Library files that exist but couldn't be read (#1507): the library isn't opened and + /// nothing is saved, so the stored copy stays as it was. + load_error: Option, } async fn boot(opts: Options) -> Boot { @@ -298,6 +301,7 @@ async fn boot(opts: Options) -> Boot { let files = Files::default(); let mut index = ThumbIndex::default(); let mut lib_dir = DEFAULT_LIBRARY_DIR.to_string(); + let mut load_error = None; if let Some(b) = &backend { if opts.reset { // everything this site keeps in the browser, photos included: never without asking @@ -315,21 +319,32 @@ async fn boot(opts: Options) -> Boot { } } // a restored backup lives in its own folder (see `backup`) - if let Ok(Some(d)) = b.read(ACTIVE_LIBRARY).await { - let d = String::from_utf8_lossy(&d).trim().to_string(); - if valid_library_dir(&d) { - lib_dir = d; + match b.read(ACTIVE_LIBRARY).await { + Ok(Some(d)) => { + let d = String::from_utf8_lossy(&d).trim().to_string(); + if valid_library_dir(&d) { + lib_dir = d; + } + } + Ok(None) => {} + // not knowing which library is in use, any library opened could be the wrong one + Err(e) => load_error = Some(format!("couldn't read which library is in use from browser storage ({e})")), + } + if load_error.is_none() { + let mut reads = Vec::new(); + for name in LIBRARY_FILES { + reads.push((name, b.read(&library_key(&lib_dir, name)).await)); + } + if let Err(e) = crate::files::preload_library(&files, reads) { + log::error!("{e}"); + load_error = Some(e); } } - for name in LIBRARY_FILES { - match b.read(&library_key(&lib_dir, name)).await { - Ok(Some(data)) => files.preload(name, data), - Ok(None) => {} - Err(e) => log::error!("reading {name}: {e}"), - } - } - if let Ok(Some(j)) = b.read(THUMB_INDEX).await { - index = ThumbIndex::from_json(&j); + let index_read = b.read(THUMB_INDEX).await; + match &index_read { + Ok(Some(j)) => index = ThumbIndex::from_json(j), + Ok(None) => {} + Err(e) => log::warn!("reading the thumbnail index: {e}"), } crate::backend::request_persistence(|granted| { if !granted { @@ -339,7 +354,8 @@ async fn boot(opts: Options) -> Boot { ); } }); - // thumbnails stored without an index entry (index saved late, or lost): delete them + // thumbnails stored without an index entry (index saved late, or lost): delete them; not + // when the index couldn't be read, which would make every thumbnail look unindexed let b2 = b.clone(); let known: std::collections::HashSet = b .list("thumbs") @@ -349,7 +365,7 @@ async fn boot(opts: Options) -> Boot { .filter(|n| n.ends_with(".jpg")) .map(|n| n.trim_end_matches(".jpg").to_string()) .collect(); - let orphans: Vec = known.into_iter().filter(|k| !index.contains(k)).collect(); + let orphans: Vec = if index_read.is_ok() { known.into_iter().filter(|k| !index.contains(k)).collect() } else { Vec::new() }; if !orphans.is_empty() { wasm_bindgen_futures::spawn_local(async move { for k in &orphans { @@ -366,7 +382,7 @@ async fn boot(opts: Options) -> Boot { index.len(), index.total() as f64 / 1e6 ); - Boot { opts, backend, files, index, lib_dir } + Boot { opts, backend, files, index, lib_dir, load_error } } struct WebApp { @@ -389,7 +405,7 @@ struct WebApp { impl WebApp { fn new(cc: &eframe::CreationContext<'_>, boot: Boot, chinese_font: Vec) -> Self { - let Boot { opts, backend, files, index, lib_dir } = boot; + let Boot { opts, backend, files, index, lib_dir, load_error } = boot; let originals = Originals::default(); let t = perf_now(); let mut session = Session::new(); @@ -401,7 +417,13 @@ impl WebApp { on_disk: false, }; let mut problem = None; - match session.open_library_in(stores, true).cloned() { + // a library that couldn't be read is not opened: it would show incomplete, then save over the stored copy + let read_failed = load_error.is_some(); + let opened = match load_error { + Some(e) => Err(format!("{e}. Nothing was changed in storage")), + None => session.open_library_in(stores, true).cloned().map_err(|e| e.to_string()), + }; + match opened { Ok(r) => log::info!( "lightcraft: library {} in {:.0} ms ({} photos; snapshot seq {}, {} ops replayed)", if r.created { "created" } else { "loaded" }, @@ -432,7 +454,8 @@ impl WebApp { } // previews are large (≤ 2560 px, f32): keep few in a 32-bit address space session.media.preview_capacity = 3; - let frozen = Rc::new(Cell::new(false)); + // nothing is saved while the stored library couldn't be read + let frozen = Rc::new(Cell::new(read_failed)); let mut app = LightcraftApp::new(session, services(originals.clone(), backend.clone(), files.clone(), frozen.clone(), cc.egui_ctx.clone())) .with_chinese_font(chinese_font); let ui_written = files.get("ui.json").unwrap_or_default(); diff --git a/docs/web.md b/docs/web.md index b275e837d..a39d9dc60 100644 --- a/docs/web.md +++ b/docs/web.md @@ -121,6 +121,11 @@ are the same in every version, so serve them with `Cache-Control: no-cache` (not writes, so commands report `saved in memory but not written`, the top bar shows the unsaved warning (as on the desktop, see `docs/control-protocol.md`), and saving is retried every 2 s until it works (`web.stats` → `saveError`). + - A failed read at start-up is not taken for a missing file: only `NotFoundError` means absent. + When a stored library file (or the `active-library` pointer) exists but can't be read, the + library isn't opened and nothing is saved, so the stored copy stays as it was; the window says + which file failed (issue #1507). A thumbnail index that can't be read keeps the stored + thumbnails instead of deleting them as unindexed. - A picked or dropped photo whose bytes can't be stored is not added (it would be gone after a reload); a notice says why. - One tab at a time: the page holds a Web Lock (`navigator.locks`, `lightcraft-library`); a diff --git a/docs/whats-new.md b/docs/whats-new.md index 97e1367a8..1b8812d1e 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,13 @@ # What's new in LightCraft +## 2026-10-15 + +### Web: an unreadable library is not opened +- In the browser version, a library file that exists but can't be read at start-up is no longer taken for a missing + one (issue #1507). LightCraft used to open the library without it, showing an incomplete catalog that the next save + would write over the stored copy. It now leaves the library unopened, saves nothing and says which file couldn't be + read; reload the page to try again. + ## 2026-10-14 ### Lightroom radial masks on the right side From d4627e9c265c191b8abd130609633b3a2392b786 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 09:01:22 +0000 Subject: [PATCH 2/2] #1507: dropping a Session no longer panics on the web `face_download::Downloads::drop` gave a running download a second to stop, with `Instant::now()` and `thread::sleep`, which panic on wasm32 ("time not implemented on this platform"). The web host replaces its Session when the library can't be opened, so with this branch an unreadable `catalog.log` killed the page with the generic panic screen instead of showing the library-problem window (found in a real-browser check of PR #1988). The wait is skipped on wasm32, where there are no download threads; the cancel flags are still set. Native test: an idle Downloads drops at once. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- crates/engine/src/face_download.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/face_download.rs b/crates/engine/src/face_download.rs index df79d5630..f5d0dd5fc 100644 --- a/crates/engine/src/face_download.rs +++ b/crates/engine/src/face_download.rs @@ -181,9 +181,16 @@ impl Drop for Downloads { for job in self.jobs.values() { job.cancel.store(true, Ordering::Relaxed); } - let started = std::time::Instant::now(); - while self.running() && started.elapsed() < Duration::from_secs(1) { - std::thread::sleep(Duration::from_millis(10)); + // Give a running download a moment to notice. Not on the web: there are no download + // threads, and `Instant::now` / `thread::sleep` panic on wasm32 ("time not implemented"), + // which killed the page when a `Session` was replaced after its library failed to open + // (issue #1507). + #[cfg(not(target_arch = "wasm32"))] + { + let started = std::time::Instant::now(); + while self.running() && started.elapsed() < Duration::from_secs(1) { + std::thread::sleep(Duration::from_millis(10)); + } } } } @@ -317,6 +324,18 @@ mod tests { } } + /// Dropping a `Downloads` with nothing running (every session has one) returns at once: the + /// wait for running threads is the only part that takes time, and the web has no threads. + #[test] + fn dropping_idle_downloads_returns_at_once() { + let t = std::time::Instant::now(); + drop(Downloads::default()); + let mut d = Downloads::default(); + d.set_outcome("m", State::Failed("x".into())); + drop(d); + assert!(t.elapsed() < Duration::from_millis(500), "{:?}", t.elapsed()); + } + #[test] fn a_running_download_shows_in_activity_and_cancels() { use crate::activity::{Activity, Unit};