diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index cb8fb96..c31a95d 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -456,8 +456,13 @@ pub async fn export_status( // worker superseded mid-run) is meaningless — surfacing its frozen `running`/77% would show a // progress bar that never moves for a keepsake nobody is building. It reads as "locked" (no // current job), which is exactly what it is. - let jobs: Vec<(String, String, i16)> = sqlx::query_as( - "SELECT j.type::text, j.status::text, j.progress_pct + // `error_message` is carried here, not just on the admin dashboard's job list. The host is the + // one who releases the keepsake and the one who owns the "Erneut versuchen" button, but this + // endpoint used to hand them a bare `failed` — so a fully actionable reason (notably the disk + // preflight's "needs X GB, Y GB free") was written to the row and then shown to nobody who + // could act on it. An admin-only diagnostic is not a diagnostic for the person on the spot. + let jobs: Vec<(String, String, i16, Option)> = sqlx::query_as( + "SELECT j.type::text, j.status::text, j.progress_pct, j.error_message FROM export_job j JOIN event e ON e.id = j.event_id WHERE e.id = $1 AND j.epoch = e.export_epoch", @@ -468,9 +473,21 @@ pub async fn export_status( let job_status = |type_name: &str| { jobs.iter() - .find(|(t, _, _)| t == type_name) - .map(|(_, status, pct)| serde_json::json!({ "status": status, "progress_pct": pct })) - .unwrap_or_else(|| serde_json::json!({ "status": "locked", "progress_pct": 0 })) + .find(|(t, _, _, _)| t == type_name) + .map(|(_, status, pct, err)| { + serde_json::json!({ + "status": status, + "progress_pct": pct, + // Only on a failure. A stale message left on a row that has since been re-armed + // would otherwise show an error next to a running progress bar. + "error_message": if status == "failed" { err.clone() } else { None }, + }) + }) + .unwrap_or_else(|| { + serde_json::json!({ + "status": "locked", "progress_pct": 0, "error_message": null, + }) + }) }; Ok(Json(serde_json::json!({ diff --git a/backend/src/services/disk.rs b/backend/src/services/disk.rs index 171ef4c..2afc197 100644 --- a/backend/src/services/disk.rs +++ b/backend/src/services/disk.rs @@ -76,6 +76,17 @@ impl Default for DiskCache { } } +/// UNCACHED free-space reading for the filesystem backing `path`. +/// +/// Deliberately bypasses [`DiskCache`]. The cache exists for the quota poll, where a 15s-stale +/// number is fine because it is only ever advisory. The export preflight is the opposite case: it +/// decides whether to start writing a multi-GB archive, and the sibling export worker running +/// concurrently can move free space by tens of gigabytes well inside the TTL. A stale reading there +/// would authorise exactly the write that fills the disk. +pub fn free_bytes(path: &Path) -> Option { + read_disk_for_path(path).map(|d| d.free) +} + /// Resolve the filesystem backing `media_path` and read its total/free bytes. /// /// Snapshots the mount table via `sysinfo`, then delegates the selection to the pure diff --git a/backend/src/services/export.rs b/backend/src/services/export.rs index 3abea0f..51099a5 100644 --- a/backend/src/services/export.rs +++ b/backend/src/services/export.rs @@ -476,6 +476,15 @@ async fn run_zip_export( return Ok(()); } + // Reclaim BEFORE measuring: the superseded archive is already unreachable, and the space it + // holds is very often exactly the space this rebuild needs. + prune_superseded_archives(pool, export_path, "Gallery", event_id, epoch).await; + // AFTER the claim, not before. A preflight that bailed before claiming would leave the row + // `pending` with no worker and no error — the spinner-forever state `mark_failed`'s status + // guard was widened to prevent. Failing here goes through the caller's `mark_failed`, so the + // host gets the reason. + ensure_export_space(pool, event_id, export_path).await?; + // On error, mark THIS generation failed — a no-op if we've since been superseded (the // caller in `spawn_export_jobs` does it, epoch-guarded). Temp artifacts are cleaned up // here so a failing export can't leak them (which is what fills the disk in the first place). @@ -658,6 +667,11 @@ async fn run_html_export( return Ok(()); } + // See run_zip_export: reclaim the superseded generation first, then refuse at the door rather + // than ENOSPC mid-write. + prune_superseded_archives(pool, export_path, "Memories", event_id, epoch).await; + ensure_export_space(pool, event_id, export_path).await?; + let res = run_html_export_inner( epoch, event_id, @@ -1161,6 +1175,182 @@ fn parse_gen_seq(name: &str, prefix: &str, suffix: &str) -> Option { .ok() } +/// Filenames a live (current-epoch, `done`) job row still points at — OFF LIMITS to every prune, +/// regardless of the epoch encoded in the name. +/// +/// A ViewerOnly regeneration carries the finished ZIP forward by re-stamping its row to the new +/// epoch WITHOUT renaming the file, so `Gallery...zip` is still the served archive and +/// deleting it by filename-epoch would 404 the download. +async fn protected_files(pool: &PgPool, event_id: Uuid) -> Vec { + sqlx::query_scalar::<_, String>( + "SELECT split_part(j.file_path, '/', -1) FROM export_job j + JOIN event e ON e.id = j.event_id + WHERE j.event_id = $1 AND j.epoch = e.export_epoch + AND j.status = 'done' AND j.file_path IS NOT NULL", + ) + .bind(event_id) + .fetch_all(pool) + .await + .unwrap_or_default() +} + +/// Reclaim superseded FINAL archives BEFORE this generation starts writing its own. +/// +/// Peak disk usage used to be two full generations, because the only prune ran after the new archive +/// was written, renamed and finalised. That ordering reads as durability ("don't delete the good +/// keepsake before the replacement is safe") but it buys nothing: readiness is derived from +/// `job.epoch = event.export_epoch AND status = 'done'`, so the moment `invalidate_and_arm` bumps +/// the epoch the old archive is ALREADY unreachable — `GET /export/zip` 404s whether the file is on +/// disk or not. Keeping it only reserves gigabytes for a download nobody can perform, and for +/// `Affects::Both` (a takedown) it is content someone has explicitly asked to have removed. So a +/// rebuild reclaims first and peaks at one generation. +/// +/// Narrower than [`prune_stale_export_files`] on purpose: FINAL archives only. Those are inert — a +/// superseded worker either already renamed its file (and will delete it itself when its guarded +/// `finalize_job` fails) or never will. `.tmp` files and `viewer_tmp_` staging dirs are NOT touched +/// here: a superseded worker can still be streaming into those, and at build START it is far more +/// likely to be alive than at finalize time. +async fn prune_superseded_archives( + pool: &PgPool, + exports_dir: &Path, + prefix: &str, + event_id: Uuid, + keep_seq: i64, +) { + let protected = protected_files(pool, event_id).await; + let final_prefix = format!("{prefix}.{event_id}."); + let mut rd = match tokio::fs::read_dir(exports_dir).await { + Ok(rd) => rd, + Err(_) => return, + }; + let mut reclaimed = 0u64; + while let Ok(Some(entry)) = rd.next_entry().await { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if !is_superseded_archive(&name, &final_prefix, keep_seq, &protected) { + continue; + } + let len = entry.metadata().await.map(|m| m.len()).unwrap_or(0); + if tokio::fs::remove_file(entry.path()).await.is_ok() { + reclaimed += len; + } + } + if reclaimed > 0 { + tracing::info!( + "reclaimed {reclaimed} bytes of superseded {prefix} archives before rebuilding \ + event {event_id} @ epoch {keep_seq}" + ); + } +} + +/// Is `name` a FINAL archive of a strictly-older generation that no live row points at? +/// +/// Pure so the two dangerous cases can be pinned without a filesystem: the carried-forward archive +/// (protected despite an older epoch in its name) and the in-flight `.tmp` (never matched at all). +fn is_superseded_archive( + name: &str, + final_prefix: &str, + keep_seq: i64, + protected: &[String], +) -> bool { + if protected.iter().any(|p| p == name) { + return false; + } + parse_gen_seq(name, final_prefix, ".zip").is_some_and(|n| n < keep_seq) +} + +/// Bytes the media in this event's keepsake will occupy, as an UPPER BOUND per archive. +/// +/// Both archives write their media entries `Compression::Stored`, so an archive is essentially a +/// byte-for-byte second copy of the originals: `Gallery.zip` always, and `Memories.zip` for every +/// video ([`MediaSource::Original`]) and every image at or under 5 MB. Images over 5 MB are +/// downscaled to 2000px first, which only makes this estimate more conservative — the direction we +/// want, since being wrong low means ENOSPC halfway through. +/// +/// Matches [`query_uploads`]' visibility filter exactly, so hidden/banned uploads aren't counted. +async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> Result { + let (bytes,): (i64,) = sqlx::query_as( + "SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint + FROM upload u + JOIN \"user\" usr ON usr.id = u.user_id + WHERE u.event_id = $1 AND u.deleted_at IS NULL + AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE", + ) + .bind(event_id) + .fetch_one(pool) + .await + .context("estimating the export size")?; + Ok(bytes.max(0) as u64) +} + +/// Headroom multiplier over the raw media sum: ZIP central directory, per-entry headers, the +/// embedded viewer, and the HTML export's temp staging. +const EXPORT_SIZE_OVERHEAD_PCT: u64 = 110; + +/// Bytes this export must have available, given the raw media sum and how many jobs are competing. +/// +/// `armed` is the multiplier that keeps two concurrent workers honest. `spawn_export_jobs` starts +/// the ZIP and the HTML halves at the same instant and both are gallery-sized, so a worker that +/// reserved only for itself would see "it fits", its sibling would independently see the same, and +/// together they would not fit — which is precisely the ENOSPC this preflight exists to prevent. +/// Computed in `u128` and clamped, NOT with `saturating_mul`: saturating first and then dividing by +/// 100 quietly turns an overflow into a number ~100x too small, which is the one direction that +/// matters here — an under-estimate authorises the very write the preflight exists to refuse. +fn required_free_bytes(media_bytes: u64, armed: i64) -> u64 { + let needed = media_bytes as u128 * EXPORT_SIZE_OVERHEAD_PCT as u128 / 100 + * armed.max(1).min(i64::from(u32::MAX)) as u128; + needed.min(u64::MAX as u128) as u64 +} + +/// Refuse to start an export that cannot fit, with a reason the host can act on. +/// +/// Without this the failure mode is ENOSPC halfway through a multi-GB write, and the wreckage +/// outlives it: the epoch has already moved, so the job row is `failed` at the CURRENT generation +/// and `GET /export/zip` 404s, while the last good archive sits on disk unreferenced. The host's +/// only escape (`POST /host/export/rebuild`) needs the very space that isn't there. Failing at the +/// door instead leaves the disk untouched and puts a number in front of the operator. +/// +/// Both halves are spawned concurrently and both are gallery-sized, so a worker must reserve for +/// its live sibling too — otherwise each independently sees "it fits", and together they don't. +async fn ensure_export_space(pool: &PgPool, event_id: Uuid, export_path: &Path) -> Result<()> { + let media_bytes = estimate_export_bytes(pool, event_id).await?; + + // Every job armed at any epoch for this event that hasn't finished is competing for this disk. + let (armed,): (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM export_job + WHERE event_id = $1 AND status IN ('pending', 'running')", + ) + .bind(event_id) + .fetch_one(pool) + .await + .context("counting armed export jobs")?; + let needed = required_free_bytes(media_bytes, armed); + + // `None` = the mount couldn't be resolved. Fail OPEN, exactly as the upload quota does: refusing + // to build the keepsake because we can't read a number would be a worse failure than trying. + let Some(free) = crate::services::disk::free_bytes(export_path) else { + tracing::warn!("export preflight: disk snapshot unavailable; proceeding without the check"); + return Ok(()); + }; + + if free < needed { + let gb = |b: u64| b as f64 / 1_000_000_000.0; + tracing::error!( + needed, + free, + armed, + "export preflight: not enough free space to build the keepsake for event {event_id}" + ); + anyhow::bail!( + "Nicht genug Speicherplatz für das Keepsake: benötigt ca. {:.1} GB, frei sind {:.1} GB. \ + Bitte Speicher freigeben und das Keepsake anschließend neu erstellen.", + gb(needed), + gb(free) + ); + } + Ok(()) +} + /// Best-effort removal of stale per-generation export artifacts for one export type. Deletes /// ONLY strictly-older generations (`n < keep_seq`) — never `keep_seq`'s own current file, /// and never a NEWER generation that a concurrent re-release may already be producing (that @@ -1177,20 +1367,7 @@ async fn prune_stale_export_files( event_id: Uuid, keep_seq: i64, ) { - // Files still referenced by a live (current-epoch) job row are OFF LIMITS regardless of the - // epoch in their name. A ViewerOnly regeneration carries the finished ZIP forward by re-stamping - // its row to the new epoch WITHOUT renaming the file — so `Gallery...zip` is still - // the served archive, and deleting it by filename-epoch would 404 the download. - let protected: Vec = sqlx::query_scalar::<_, String>( - "SELECT split_part(j.file_path, '/', -1) FROM export_job j - JOIN event e ON e.id = j.event_id - WHERE j.event_id = $1 AND j.epoch = e.export_epoch - AND j.status = 'done' AND j.file_path IS NOT NULL", - ) - .bind(event_id) - .fetch_all(pool) - .await - .unwrap_or_default(); + let protected = protected_files(pool, event_id).await; // EVERY shape is event-scoped. All events share one exports volume, so a name keyed only by // generation would let event A's prune delete event B's live keepsake (and let two events @@ -1504,3 +1681,131 @@ So geht's:\n\ Alles ist lokal auf deinem Gerät gespeichert.\n\ \n\ Viel Freude mit den Erinnerungen!\n"; + +// ── Tests ──────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + const EVT: &str = "11111111-1111-1111-1111-111111111111"; + + fn gallery_prefix() -> String { + format!("Gallery.{EVT}.") + } + + #[test] + fn a_strictly_older_archive_is_reclaimed() { + // The whole point: at the start of a rebuild at epoch 5, generation 4's archive is dead + // weight — readiness is derived from `epoch = event.export_epoch`, so it is already + // unreachable — and its bytes are very often exactly the bytes the rebuild needs. + assert!(is_superseded_archive( + &format!("Gallery.{EVT}.4.zip"), + &gallery_prefix(), + 5, + &[] + )); + } + + #[test] + fn our_own_and_newer_generations_are_never_touched() { + // `keep_seq` is OUR generation; a NEWER one belongs to a re-release that has already + // superseded us, and deleting it would let a lagging worker nuke a live keepsake. + for seq in [5, 6] { + assert!( + !is_superseded_archive( + &format!("Gallery.{EVT}.{seq}.zip"), + &gallery_prefix(), + 5, + &[] + ), + "generation {seq} must survive a prune keeping 5" + ); + } + } + + #[test] + fn a_carried_forward_archive_survives_despite_an_older_epoch_in_its_name() { + // THE dangerous case. A ViewerOnly regeneration (a moderated comment) re-stamps the + // finished ZIP's row to the new epoch WITHOUT renaming the file, so the SERVED archive + // legitimately carries an older generation number. Pruning it by filename-epoch would 404 + // the photo download to change nothing in it. The protected set is what stops that, and + // moving the prune to build-start makes this case reachable far more often. + let carried = format!("Gallery.{EVT}.4.zip"); + assert!(!is_superseded_archive( + &carried, + &gallery_prefix(), + 5, + std::slice::from_ref(&carried) + )); + } + + #[test] + fn temps_and_staging_dirs_are_out_of_scope_for_the_early_prune() { + // A superseded worker may still be streaming into these, and at build START it is much + // more likely to be alive than at finalize time. Only inert FINAL archives are reclaimed + // here; `prune_stale_export_files` still handles the rest after we win. + for name in [ + format!("Gallery.{EVT}.4.tmp"), + format!("viewer_tmp_{EVT}_4"), + format!("Memories.{EVT}.4.zip"), + ] { + assert!( + !is_superseded_archive(&name, &gallery_prefix(), 5, &[]), + "{name} must not be reclaimed by the pre-build prune" + ); + } + } + + #[test] + fn another_events_archive_is_never_reclaimed() { + // All events share one exports volume, so the prefix carries the event id. + let other = "22222222-2222-2222-2222-222222222222"; + assert!(!is_superseded_archive( + &format!("Gallery.{other}.4.zip"), + &gallery_prefix(), + 5, + &[] + )); + } + + #[test] + fn unrelated_files_are_left_alone() { + for name in ["Gallery.zip", "notes.txt", "Gallery..4.zip"] { + assert!(!is_superseded_archive(name, &gallery_prefix(), 5, &[])); + } + } + + #[test] + fn a_lone_armed_job_reserves_for_one_archive() { + // A ViewerOnly regeneration re-arms only the HTML half — reserving for two would refuse + // a rebuild that fits perfectly well. + assert_eq!(required_free_bytes(1_000, 1), 1_100); + } + + #[test] + fn two_concurrent_halves_reserve_for_both() { + // The bug this exists to prevent: each worker independently sees "it fits", and together + // they don't. Both halves are gallery-sized, so the reservation must be for the pair. + assert_eq!(required_free_bytes(1_000, 2), 2_200); + } + + #[test] + fn a_zero_count_still_reserves_for_one() { + // Defensive: a racing status transition must never yield a zero requirement, which would + // wave through an export of any size onto a full disk. + assert_eq!(required_free_bytes(1_000, 0), 1_100); + } + + #[test] + fn an_empty_gallery_needs_nothing() { + assert_eq!(required_free_bytes(0, 2), 0); + } + + #[test] + fn a_pathological_size_saturates_instead_of_wrapping() { + // u64 overflow would wrap to a TINY requirement and authorise the exact write we're + // guarding against — the failure mode must be "refuse", never "wrap and allow". + assert_eq!(required_free_bytes(u64::MAX, 2), u64::MAX); + } +} diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index c0a0a6b..9420b23 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -255,3 +255,85 @@ pub async fn downloadable(pool: &PgPool, event_id: Uuid, export_type: &str) -> O .expect("downloadable") .flatten() } + +/// Insert an upload of `size` bytes, optionally already soft-deleted. +pub async fn seed_upload( + pool: &PgPool, + event_id: Uuid, + user_id: Uuid, + size: i64, + deleted: bool, +) -> Uuid { + sqlx::query_scalar( + "INSERT INTO upload (event_id, user_id, original_path, mime_type, + original_size_bytes, deleted_at) + VALUES ($1, $2, 'originals/x.jpg', 'image/jpeg', $3, + CASE WHEN $4 THEN NOW() ELSE NULL END) + RETURNING id", + ) + .bind(event_id) + .bind(user_id) + .bind(size) + .bind(deleted) + .fetch_one(pool) + .await + .expect("seed upload") +} + +/// Flip the moderation flags a ban sets. +pub async fn set_user_moderation(pool: &PgPool, user_id: Uuid, banned: bool, hidden: bool) { + sqlx::query("UPDATE \"user\" SET is_banned = $2, uploads_hidden = $3 WHERE id = $1") + .bind(user_id) + .bind(banned) + .bind(hidden) + .execute(pool) + .await + .expect("set moderation"); +} + +/// SRC: `services/export.rs::query_uploads` — the visibility filter, verbatim, projected down to +/// `(id, original_size_bytes)`. This is the row set that ACTUALLY lands in the archives. +pub async fn export_visible_uploads(pool: &PgPool, event_id: Uuid) -> Vec<(Uuid, i64)> { + sqlx::query_as( + "SELECT u.id, u.original_size_bytes + FROM upload u + JOIN \"user\" usr ON usr.id = u.user_id + WHERE u.event_id = $1 AND u.deleted_at IS NULL + AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE + GROUP BY u.id, usr.display_name + ORDER BY u.created_at ASC", + ) + .bind(event_id) + .fetch_all(pool) + .await + .expect("export_visible_uploads") +} + +/// SRC: `services/export.rs::estimate_export_bytes` — verbatim. +pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> i64 { + let (bytes,): (i64,) = sqlx::query_as( + "SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint + FROM upload u + JOIN \"user\" usr ON usr.id = u.user_id + WHERE u.event_id = $1 AND u.deleted_at IS NULL + AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE", + ) + .bind(event_id) + .fetch_one(pool) + .await + .expect("estimate_export_bytes"); + bytes +} + +/// SRC: `services/export.rs::ensure_export_space` — the armed-job count, verbatim. +pub async fn armed_job_count(pool: &PgPool, event_id: Uuid) -> i64 { + let (n,): (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM export_job + WHERE event_id = $1 AND status IN ('pending', 'running')", + ) + .bind(event_id) + .fetch_one(pool) + .await + .expect("armed_job_count"); + n +} diff --git a/backend/tests/export_preflight.rs b/backend/tests/export_preflight.rs new file mode 100644 index 0000000..4f556fc --- /dev/null +++ b/backend/tests/export_preflight.rs @@ -0,0 +1,154 @@ +//! DB-backed tests for the export disk preflight. +//! +//! The keepsake used to be built with NO free-space check at all, and the failure that produced was +//! not "the export failed" but "the deliverable is stuck and the escape hatch needs the space that +//! isn't there": +//! +//! 1. A takedown bumps the epoch and re-arms both halves. +//! 2. The ZIP hits ENOSPC partway through a multi-GB write. +//! 3. The job row is now `failed` at the CURRENT epoch, so readiness +//! (`epoch = event.export_epoch AND status = 'done'`) is false and `GET /export/zip` 404s — +//! while the last good archive sits on disk, unreferenced and unreachable. +//! 4. `POST /host/export/rebuild` re-arms the same doomed write. +//! +//! Two changes close it: reclaim the superseded generation BEFORE building (so peak usage is one +//! generation, not two) and refuse up front with a number the host can act on. +//! +//! What these tests pin is the ESTIMATE — the part that decides. The arithmetic on top of it lives +//! in `services/export.rs`'s unit tests; the filesystem selection lives in `is_superseded_archive`. +//! The risk here is drift: if `query_uploads` ever gains or loses a visibility predicate and +//! `estimate_export_bytes` doesn't, the preflight silently sizes the wrong gallery. So rather than +//! restating the filter, these assert the estimate against the row set the archive actually +//! contains. + +mod common; + +use common::*; +use sqlx::PgPool; + +/// The estimate must equal the sum over EXACTLY the rows `query_uploads` returns — computed from +/// that row set, not from a restatement of its WHERE clause. +/// +/// PREVENTS: the two queries drifting apart. An estimate that counts rows the archive skips is +/// merely pessimistic; one that MISSES rows the archive writes under-reserves, which is the whole +/// failure being guarded against. +#[sqlx::test] +async fn the_estimate_sums_exactly_the_rows_the_archive_will_contain(pool: PgPool) { + let event_id = seed_event(&pool, "wedding").await; + + let visible = seed_user(&pool, event_id, "Anna").await; + let banned = seed_user(&pool, event_id, "Ben").await; + let hidden = seed_user(&pool, event_id, "Cara").await; + + seed_upload(&pool, event_id, visible, 1_000, false).await; + seed_upload(&pool, event_id, visible, 2_500, false).await; + // Each of these is excluded from the archive by a DIFFERENT predicate. + seed_upload(&pool, event_id, visible, 9_000, true).await; // soft-deleted + seed_upload(&pool, event_id, banned, 9_000, false).await; // uploader banned + seed_upload(&pool, event_id, hidden, 9_000, false).await; // uploads hidden + + set_user_moderation(&pool, banned, true, true).await; + set_user_moderation(&pool, hidden, false, true).await; + + let rows = export_visible_uploads(&pool, event_id).await; + let expected: i64 = rows.iter().map(|(_, bytes)| bytes).sum(); + + assert_eq!(rows.len(), 2, "only Anna's two live uploads are archived"); + assert_eq!( + estimate_export_bytes(&pool, event_id).await, + expected, + "the preflight must size the gallery the export will actually write" + ); + assert_eq!(expected, 3_500); +} + +/// An event with nothing to archive estimates zero rather than NULL. +/// +/// PREVENTS: `SUM()` over no rows returning NULL and the decode blowing up — which would abort the +/// export with a type error instead of building an (entirely legitimate) empty keepsake. +#[sqlx::test] +async fn an_empty_gallery_estimates_zero_not_null(pool: PgPool) { + let event_id = seed_event(&pool, "wedding").await; + assert_eq!(estimate_export_bytes(&pool, event_id).await, 0); + + // And with a user who has uploaded nothing. + seed_user(&pool, event_id, "Anna").await; + assert_eq!(estimate_export_bytes(&pool, event_id).await, 0); +} + +/// A release arms both halves, so the preflight sees a count of 2 and reserves for the pair. +/// +/// PREVENTS: the concurrency under-reservation. `spawn_export_jobs` starts the ZIP and HTML workers +/// at the same instant, and BOTH are gallery-sized (`Memories.zip` streams the original for every +/// video and every image at or under 5 MB, all `Compression::Stored`). A worker reserving only for +/// itself would see "it fits", its sibling would independently see the same, and together they +/// would ENOSPC — which is why `required_free_bytes` multiplies by this count. +#[sqlx::test] +async fn a_release_arms_both_halves_so_the_preflight_reserves_for_two(pool: PgPool) { + let event_id = seed_event(&pool, "wedding").await; + let user = seed_user(&pool, event_id, "Anna").await; + seed_upload(&pool, event_id, user, 1_000, false).await; + + assert_eq!( + armed_job_count(&pool, event_id).await, + 0, + "nothing is armed before the release" + ); + + let epoch = release_gallery(&pool, "wedding").await.expect("released"); + assert_eq!( + armed_job_count(&pool, event_id).await, + 2, + "a release arms zip AND html — both compete for the same disk" + ); + + // A worker that has claimed its half is still competing; `running` must keep counting. + assert!(claim_job(&pool, event_id, "zip", epoch).await); + assert_eq!( + armed_job_count(&pool, event_id).await, + 2, + "claiming moves pending -> running, which must not drop out of the reservation" + ); + + // Only a FINISHED half stops competing. + assert!(finalize_job(&pool, event_id, "zip", epoch, "exports/Gallery.zip").await); + assert_eq!( + armed_job_count(&pool, event_id).await, + 1, + "a done half no longer needs space reserved for it" + ); +} + +/// A ViewerOnly regeneration re-arms only the HTML half, so the preflight reserves for one. +/// +/// PREVENTS: over-reservation refusing a rebuild that fits perfectly well. Moderating a comment +/// carries the finished ZIP forward untouched; demanding room for a second copy of it would fail +/// the one operation that needs no new gallery-sized write at all. +#[sqlx::test] +async fn a_viewer_only_regeneration_reserves_for_one_half(pool: PgPool) { + let event_id = seed_event(&pool, "wedding").await; + let user = seed_user(&pool, event_id, "Anna").await; + seed_upload(&pool, event_id, user, 1_000, false).await; + + let epoch = release_gallery(&pool, "wedding").await.expect("released"); + for t in ["zip", "html"] { + assert!(claim_job(&pool, event_id, t, epoch).await); + assert!(finalize_job(&pool, event_id, t, epoch, &format!("exports/{t}")).await); + } + assert_eq!(armed_job_count(&pool, event_id).await, 0); + + // A moderated comment: bump the epoch, carry the ZIP forward, re-arm only the viewer. + let (_, _, next) = bump_epoch(&pool, "wedding").await.expect("bumped"); + assert!( + carry_zip_forward(&pool, event_id, next).await, + "the finished ZIP is re-stamped, not rebuilt" + ); + let mut conn = pool.acquire().await.expect("acquire"); + enqueue_types_at_epoch(&mut conn, event_id, next, &["html"]).await; + + assert_eq!( + armed_job_count(&pool, event_id).await, + 1, + "only the viewer is being rebuilt, so only one archive's worth of space is needed" + ); +} diff --git a/e2e/fixtures/db.ts b/e2e/fixtures/db.ts index cdbe6e2..7db267d 100644 --- a/e2e/fixtures/db.ts +++ b/e2e/fixtures/db.ts @@ -142,7 +142,8 @@ export const db = { async fakeExportJob( eventSlug: string, type: 'zip' | 'html', - status: 'pending' | 'running' | 'done' + status: 'pending' | 'running' | 'done' | 'failed', + errorMessage: string | null = null ) { await withClient(async (c) => { const ev = await c.query<{ id: string; export_epoch: string }>( @@ -151,11 +152,12 @@ export const db = { ); if (ev.rows.length === 0) throw new Error(`No event with slug ${eventSlug}`); await c.query( - `INSERT INTO export_job (event_id, type, status, progress_pct, completed_at, epoch) - VALUES ($1, $2::export_type, $3::export_status, $4, $5, $6) + `INSERT INTO export_job (event_id, type, status, progress_pct, completed_at, epoch, + error_message) + VALUES ($1, $2::export_type, $3::export_status, $4, $5, $6, $7) ON CONFLICT (event_id, type) DO UPDATE SET status = EXCLUDED.status, progress_pct = EXCLUDED.progress_pct, - epoch = EXCLUDED.epoch`, + epoch = EXCLUDED.epoch, error_message = EXCLUDED.error_message`, [ ev.rows[0].id, type, @@ -163,6 +165,7 @@ export const db = { status === 'done' ? 100 : 0, status === 'done' ? new Date() : null, ev.rows[0].export_epoch, + errorMessage, ] ); }); diff --git a/e2e/specs/06-export/failure-reason-visible.spec.ts b/e2e/specs/06-export/failure-reason-visible.spec.ts new file mode 100644 index 0000000..72c8535 --- /dev/null +++ b/e2e/specs/06-export/failure-reason-visible.spec.ts @@ -0,0 +1,95 @@ +/** + * Regression guard — when the keepsake fails to build, the HOST must be told why. + * + * `/export/status` reported `{status, progress_pct}` and nothing else, so the host dashboard could + * only ever render "Keepsake-Erstellung fehlgeschlagen." next to an "Erneut versuchen" button. The + * reason WAS being written — `mark_failed` stores it on the job row — but it surfaced solely in the + * admin dashboard's job list. The host is the person who releases the gallery, owns the retry + * button, and is standing at the venue; the admin may be someone else entirely, or the same person + * without the password to hand. + * + * That matters most for the failure this shipped alongside: the export disk preflight. Its message + * names the two numbers that decide what to do ("benötigt ca. X GB, frei sind Y GB"), and without + * it "Erneut versuchen" fails identically, forever, with no hint that the answer is free some space. + * + * These drive the real UI and the real endpoint — the plumbing is four hops (SQL → handler JSON → + * store type → Svelte branch) and any one of them dropping the field restores the silent version. + */ +import { test, expect } from '../../fixtures/test'; +import { BASE } from '../../helpers/env'; + +const SLUG = 'e2e-test-event'; +const DISK_REASON = + 'Nicht genug Speicherplatz für das Keepsake: benötigt ca. 42.0 GB, frei sind 3.0 GB. ' + + 'Bitte Speicher freigeben und das Keepsake anschließend neu erstellen.'; + +test.describe('Export — a failed keepsake explains itself to the host', () => { + test('the failure reason reaches /export/status', async ({ host, db }) => { + await db.setExportReleased(SLUG, true); + await db.fakeExportJob(SLUG, 'zip', 'failed', DISK_REASON); + await db.fakeExportJob(SLUG, 'html', 'failed', DISK_REASON); + + const res = await fetch(`${BASE}/api/v1/export/status`, { + headers: { Authorization: `Bearer ${host.jwt}` }, + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { + zip: { status: string; error_message: string | null }; + html: { status: string; error_message: string | null }; + }; + + expect(body.zip.status).toBe('failed'); + expect( + body.zip.error_message, + 'the reason must travel with the status, not live only in the admin job list' + ).toBe(DISK_REASON); + expect(body.html.error_message).toBe(DISK_REASON); + }); + + test('a succeeding export carries no stale reason', async ({ host, db }) => { + // The mirror that keeps the above honest: a handler that returned `error_message` + // unconditionally would pass the first test while showing an error next to a green + // "Keepsake ist bereit." Rows keep their last message until they are re-armed, so this is a + // real state, not a hypothetical one. + await db.setExportReleased(SLUG, true); + await db.fakeExportJob(SLUG, 'zip', 'failed', DISK_REASON); + await db.fakeExportJob(SLUG, 'zip', 'done', DISK_REASON); + + const res = await fetch(`${BASE}/api/v1/export/status`, { + headers: { Authorization: `Bearer ${host.jwt}` }, + }); + const body = (await res.json()) as { zip: { status: string; error_message: string | null } }; + expect(body.zip.status).toBe('done'); + expect( + body.zip.error_message, + 'a message left on a row that has since succeeded must not be shown' + ).toBeNull(); + }); + + test('the host dashboard renders the reason under the failure', async ({ + page, + host, + signIn, + db, + }) => { + await db.setExportReleased(SLUG, true); + await db.fakeExportJob(SLUG, 'zip', 'failed', DISK_REASON); + await db.fakeExportJob(SLUG, 'html', 'failed', DISK_REASON); + + await signIn(page, host); + await page.goto('/host'); + + await expect(page.getByText(/Keepsake-Erstellung fehlgeschlagen/i)).toBeVisible({ + timeout: 15_000, + }); + // The actionable half — the numbers, not just the verdict. + await expect( + page.getByText(/Nicht genug Speicherplatz/i), + 'the host must see WHY, next to the only button they have' + ).toBeVisible(); + await expect(page.getByText(/3\.0 GB/)).toBeVisible(); + + // And the retry button is still mounted — it is deliberately outside the status branches. + await expect(page.getByTestId('export-rebuild')).toBeVisible(); + }); +}); diff --git a/frontend/src/lib/export-status-store.ts b/frontend/src/lib/export-status-store.ts index d9e3d2d..0d9965b 100644 --- a/frontend/src/lib/export-status-store.ts +++ b/frontend/src/lib/export-status-store.ts @@ -11,6 +11,8 @@ import { getToken, onClearAuth } from './auth'; export interface ExportJob { status: 'locked' | 'pending' | 'running' | 'done' | 'failed'; progress_pct: number; + /** Only populated when `status === 'failed'` — the reason, phrased for the host. */ + error_message: string | null; } export interface ExportStatusSnapshot { diff --git a/frontend/src/routes/host/+page.svelte b/frontend/src/routes/host/+page.svelte index 8212c5f..f485927 100644 --- a/frontend/src/routes/host/+page.svelte +++ b/frontend/src/routes/host/+page.svelte @@ -40,6 +40,7 @@ interface ExportJob { status: string; progress_pct: number; + error_message: string | null; } interface ExportStatusDto { released: boolean; @@ -71,6 +72,11 @@ let exportProgress = $derived( Math.min(exportInfo?.zip?.progress_pct ?? 0, exportInfo?.html?.progress_pct ?? 0) ); + // Either half can carry the reason, and a disk failure usually fails both with the same text — + // so take the first one present rather than rendering it twice. + let exportError = $derived( + exportInfo?.zip?.error_message ?? exportInfo?.html?.error_message ?? null + ); // SSE unsubscribers, torn down on destroy. let sseOff: Array<() => void> = []; @@ -686,6 +692,13 @@

{:else}

Keepsake-Erstellung fehlgeschlagen.

+ + {#if exportError} +

{exportError}

+ {/if} {/if}