mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-26 19:17:02 -06:00
#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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK
This commit is contained in:
@@ -22,6 +22,12 @@ fn err(e: JsValue) -> String {
|
||||
e.as_string().or_else(|| e.dyn_ref::<js_sys::Error>().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<web_sys::StorageManager> {
|
||||
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)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Option<Vec<u8>>, 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();
|
||||
|
||||
@@ -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<String>,
|
||||
}
|
||||
|
||||
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<String> = 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<String> = known.into_iter().filter(|k| !index.contains(k)).collect();
|
||||
let orphans: Vec<String> = 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<u8>) -> 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();
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user