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 4b5f6289d..9cd934ce5 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::*; @@ -308,6 +329,40 @@ mod tests { assert!(unsaved_refusal(Some((3, "quota"))).unwrap().contains("3 changes could not")); } + /// 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 3ce8eb66c..56bd3210b 100644 --- a/apps/lightcraft-web/src/web.rs +++ b/apps/lightcraft-web/src/web.rs @@ -283,6 +283,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 { @@ -301,6 +304,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 @@ -318,21 +322,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 { @@ -342,7 +357,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") @@ -352,7 +368,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 { @@ -369,7 +385,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 { @@ -392,7 +408,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(); @@ -404,7 +420,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" }, @@ -435,7 +457,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/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}; diff --git a/docs/web.md b/docs/web.md index a755bfd80..98d0dd91c 100644 --- a/docs/web.md +++ b/docs/web.md @@ -123,6 +123,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 178c34ee3..07502e012 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -8,6 +8,12 @@ now refuses to start and says how many changes are unsaved and why. LightCraft retries the save with every change; back up once nothing is left unsaved. +### 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 ### Model downloads time out on a name that doesn't resolve