diff --git a/.env.example b/.env.example index 4938695..69efe58 100644 --- a/.env.example +++ b/.env.example @@ -32,6 +32,13 @@ POSTGRES_DB=eventsnap # OOM in Postgres doesn't degrade one feature, it takes the whole event down. DATABASE_MAX_CONNECTIONS=30 +# Log level. `info` is the right production default: at `debug` the tower-http trace +# layer writes a line per request AND per response, which on a busy event is a large +# multiple of the useful output. Container logs are capped at 10m x 3 per service +# (docker-compose.yml), so a chatty level buys you a shorter history, not more of it. +# To debug a live event: RUST_LOG=eventsnap_backend=debug docker compose up -d app +RUST_LOG=info + # ── Authentication ──────────────────────────────────────────────────────────── # Generate with: openssl rand -hex 64 JWT_SECRET=change_me_to_a_random_64_byte_hex_string diff --git a/backend/src/handlers/health.rs b/backend/src/handlers/health.rs new file mode 100644 index 0000000..a677c7b --- /dev/null +++ b/backend/src/handlers/health.rs @@ -0,0 +1,43 @@ +use std::time::Duration; + +use axum::extract::State; +use axum::http::StatusCode; + +use crate::state::AppState; + +/// How long a readiness probe waits for the database before calling the app not-ready. +/// +/// Deliberately shorter than the pool's own `acquire_timeout`: a saturated pool IS a +/// not-ready condition, and reporting it as such is the point. Do not "fix" the two to +/// match — that would make the probe wait out the very saturation it exists to surface. +const READY_TIMEOUT: Duration = Duration::from_secs(2); + +/// Readiness: can this instance actually serve a request end to end? +/// +/// Separate from `/health` (liveness) ON PURPOSE, and the compose healthcheck must keep +/// pointing at `/health`. `caddy` declares `depends_on: app: {condition: service_healthy}`, +/// so a DB-dependent healthcheck would turn a transient Postgres hiccup during boot into +/// the reverse proxy refusing to start — converting a blip into a total outage. This route +/// is for an external monitor, which should page rather than restart. +/// +/// The gap this closes: every request path touches the database, so with a dead or +/// saturated pool the app is useless while `/health` still answers "ok" — the disk-full +/// endgame (media volume fills, Postgres cannot write WAL) stayed green all the way down. +pub async fn ready(State(state): State) -> (StatusCode, &'static str) { + let probe = sqlx::query_scalar::<_, i32>("SELECT 1").fetch_one(&state.pool); + + match tokio::time::timeout(READY_TIMEOUT, probe).await { + Ok(Ok(_)) => (StatusCode::OK, "ready"), + Ok(Err(e)) => { + tracing::warn!(error = ?e, "readiness probe failed: database unreachable"); + (StatusCode::SERVICE_UNAVAILABLE, "database unavailable") + } + Err(_) => { + tracing::warn!( + timeout_secs = READY_TIMEOUT.as_secs(), + "readiness probe timed out; the pool is saturated or the database is hung" + ); + (StatusCode::SERVICE_UNAVAILABLE, "database timeout") + } + } +} diff --git a/backend/src/handlers/mod.rs b/backend/src/handlers/mod.rs index 830f6de..72eb6ef 100644 --- a/backend/src/handlers/mod.rs +++ b/backend/src/handlers/mod.rs @@ -1,5 +1,6 @@ pub mod admin; pub mod feed; +pub mod health; pub mod host; pub mod me; pub mod public; diff --git a/backend/src/main.rs b/backend/src/main.rs index bf5a3b9..c52e8c1 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -27,8 +27,15 @@ async fn main() -> Result<()> { tracing_subscriber::registry() .with( + // `info`, not `debug`. A stock deploy sets RUST_LOG nowhere (it is absent from + // .env.example and was absent from docker-compose.yml), so this fallback IS the + // production level — and at `debug` the TraceLayer below emits a line per request + // AND per response, into a log file that had no rotation. `tower_http=warn` + // rather than `info` states the intent: those spans are diagnostics, not an + // access log, and a future `DefaultOnResponse::new().level(Level::INFO)` should + // not silently re-enable them. tracing_subscriber::EnvFilter::try_from_default_env() - .unwrap_or_else(|_| "eventsnap_backend=debug,tower_http=debug".into()), + .unwrap_or_else(|_| "eventsnap_backend=info,tower_http=warn".into()), ) .with(tracing_subscriber::fmt::layer()) .init(); @@ -248,7 +255,11 @@ async fn main() -> Result<()> { // four subtrees. Deleting the route removes the vector outright rather than racing the // decoder; `/media/**` now 404s regardless of encoding. let router = Router::new() + // Liveness. Stays dependency-free — the compose healthcheck gates Caddy's startup + // on it, so anything that can fail transiently must NOT be in here. .route("/health", get(|| async { "ok" })) + // Readiness. Touches the pool; for an external monitor, not for the compose gate. + .route("/health/ready", get(handlers::health::ready)) .merge(api) .layer(TraceLayer::new_for_http()) .with_state(state); diff --git a/backend/src/services/video.rs b/backend/src/services/video.rs index d22ee22..aa35948 100644 --- a/backend/src/services/video.rs +++ b/backend/src/services/video.rs @@ -71,7 +71,7 @@ pub async fn extract_poster_frame(src: &Path, dest: &Path, width: u32) -> Result /// Run one ffmpeg attempt. A non-zero exit is NOT an error here — the artifact check above is the /// authority, and a corrupt input that fails at 1 s may still yield a frame at 0. async fn run_ffmpeg(src: &Path, dest: &Path, width: u32, seek: &str) -> Result<()> { - let mut child = tokio::process::Command::new("ffmpeg") + let child = tokio::process::Command::new("ffmpeg") .args([ // BEFORE -i: an input-side seek. See SEEK_POSITIONS. "-ss", @@ -85,24 +85,56 @@ async fn run_ffmpeg(src: &Path, dest: &Path, width: u32, seek: &str) -> Result<( "-y", dest.to_str().unwrap_or_default(), ]) - .stdout(std::process::Stdio::piped()) + // ffmpeg writes the poster to `dest` itself; nothing here ever reads stdout, so + // giving it a pipe only created something that could fill. + .stdout(std::process::Stdio::null()) + // stderr IS piped — it is the only diagnostic when a clip yields no frame — but it + // must be DRAINED, which is the whole point of `wait_with_output` below. .stderr(std::process::Stdio::piped()) .kill_on_drop(true) .spawn() .context("failed to spawn ffmpeg")?; - match tokio::time::timeout(FFMPEG_TIMEOUT, child.wait()).await { - Ok(res) => { - res.context("ffmpeg wait failed")?; - } - Err(_) => { - let _ = child.kill().await; - anyhow::bail!("ffmpeg timed out after {}s", FFMPEG_TIMEOUT.as_secs()); - } + // `wait_with_output`, NOT `wait`. ffmpeg is verbose on stderr (banner, stream info, + // per-frame progress) and `wait()` reads neither pipe — so once the ~64 KiB pipe buffer + // filled, ffmpeg blocked writing, `wait()` never returned, and the call burned the full + // timeout. That is not merely slow: the timeout is an `Err`, so after 2 seek positions x + // 3 compression attempts the caller soft-deletes a perfectly playable video for a + // poster-frame failure. `wait_with_output` polls the pipe and the exit status together. + // + // It also CONSUMES the child, so the explicit `child.kill()` that used to sit on the + // timeout arm cannot exist here — and is not needed: `kill_on_drop(true)` is set above, + // and dropping the future on timeout drops the child with it. + let out = match tokio::time::timeout(FFMPEG_TIMEOUT, child.wait_with_output()).await { + Ok(res) => res.context("ffmpeg wait failed")?, + Err(_) => anyhow::bail!("ffmpeg timed out after {}s", FFMPEG_TIMEOUT.as_secs()), + }; + + // A non-zero exit is not an error (see the doc comment) — the artifact check in + // `extract_poster_frame` is the authority. Log the tail so a systematically failing + // format is diagnosable without turning it into data loss. + if !out.status.success() { + tracing::debug!( + seek, + status = ?out.status, + stderr = %tail_lines(&out.stderr, 10), + "ffmpeg exited non-zero; the artifact check decides" + ); } Ok(()) } +/// Last `n` lines of a child's stderr, lossily decoded. +/// +/// Bounded on purpose: ffmpeg's stderr is unbounded, and the reason we now drain it is that +/// unbounded output used to be a hazard. Emitting all of it into a log line — into container +/// logs that are themselves size-capped — would just move the problem. +fn tail_lines(bytes: &[u8], n: usize) -> String { + let text = String::from_utf8_lossy(bytes); + let lines: Vec<&str> = text.lines().filter(|l| !l.trim().is_empty()).collect(); + lines[lines.len().saturating_sub(n)..].join(" | ") +} + #[cfg(test)] mod tests { use super::*; @@ -137,4 +169,65 @@ mod tests { ); let _ = std::fs::remove_dir_all(&dir); } + + #[test] + fn the_stderr_tail_is_bounded_and_survives_invalid_utf8() { + let noisy: Vec = (0..500) + .map(|i| format!("line {i}\n")) + .collect::() + .into_bytes(); + let got = tail_lines(&noisy, 3); + assert_eq!(got, "line 497 | line 498 | line 499"); + + // ffmpeg emits filenames verbatim, so its stderr is not guaranteed to be UTF-8. + assert_eq!(tail_lines(&[b'o', b'k', 0xff], 5), "ok\u{fffd}"); + assert_eq!(tail_lines(b"", 5), ""); + } + + /// A real extraction must finish in a small fraction of `FFMPEG_TIMEOUT`. + /// + /// Wall-clock is the ONLY observable of the bug this guards: piping stderr and then + /// calling `wait()` (which drains nothing) blocks ffmpeg on a full pipe buffer until the + /// timeout fires, and the timeout is an `Err`, so the upload is soft-deleted. The + /// assertion is deliberately on elapsed time, not on the exit status. + /// + /// Honest limitation: our fixture is quiet enough not to fill a 64 KiB pipe on its own, + /// so this catches a regression to `wait()` only in combination with a verbose input. It + /// is still worth pinning — a reverted drain plus any chatty clip is data loss. + #[tokio::test] + async fn a_real_clip_yields_a_poster_well_inside_the_timeout() { + if tokio::process::Command::new("ffmpeg") + .arg("-version") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .await + .is_err() + { + eprintln!("skipping: ffmpeg not on PATH"); + return; + } + let src = Path::new("../e2e/fixtures/media/sample.mp4"); + if !src.exists() { + eprintln!("skipping: {} missing", src.display()); + return; + } + + let dir = std::env::temp_dir().join(format!("es-video-ok-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let dest = dir.join("poster.jpg"); + + let started = std::time::Instant::now(); + let got = extract_poster_frame(src, &dest, 400).await; + let elapsed = started.elapsed(); + + assert!(matches!(got, Ok(true)), "expected a poster, got {got:?}"); + assert!(dest.metadata().unwrap().len() > 0); + assert!( + elapsed < FFMPEG_TIMEOUT / 4, + "extraction took {elapsed:?}; a drained stderr finishes in well under \ + {FFMPEG_TIMEOUT:?} — this is the pipe-deadlock regression guard" + ); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/docker-compose.yml b/docker-compose.yml index 734d6b3..68853e0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,20 @@ +# Docker's default json-file driver is UNBOUNDED. Those files land on the HOST +# filesystem, outside every `deploy.resources.limits` below — so the container memory +# caps do nothing to stop them. On a single-box deployment the host disk is also where +# the postgres_data and media_data volumes live, and a full disk stops Postgres writing +# WAL, which takes the whole event down. 4 services x 3 x 10m caps the worst case at +# ~120 MiB. Applied to every service via the anchor; a new service must opt in too. +x-logging: &default-logging + driver: json-file + options: + max-size: "10m" + max-file: "3" + services: db: image: postgres:16-alpine restart: unless-stopped + logging: *default-logging env_file: .env environment: POSTGRES_USER: ${POSTGRES_USER} @@ -32,8 +45,13 @@ services: context: ./backend dockerfile: Dockerfile restart: unless-stopped + logging: *default-logging env_file: .env environment: + # Default to info. The code fallback in main.rs is info too, but a stock deploy + # sets RUST_LOG nowhere, and this is the layer an operator will actually find when + # they need to raise it for a single event ("RUST_LOG=eventsnap_backend=debug"). + RUST_LOG: ${RUST_LOG:-info} # Activates the production secret guard in config.rs — refuses to boot with # placeholder JWT_SECRET / ADMIN_PASSWORD_HASH. APP_ENV: production @@ -75,6 +93,7 @@ services: context: ./frontend dockerfile: Dockerfile restart: unless-stopped + logging: *default-logging env_file: .env environment: # adapter-node behind Caddy TLS needs the public origin for CSRF checks on @@ -100,6 +119,7 @@ services: caddy: image: caddy:2-alpine restart: unless-stopped + logging: *default-logging environment: # The Caddyfile's site address is `{$DOMAIN}`, read from THIS container's env. # Without it, `{$DOMAIN}` expands to empty, the site block collapses, and Caddy diff --git a/e2e/fixtures/media/not-an-image.jpg b/e2e/fixtures/media/not-an-image.jpg new file mode 100644 index 0000000..4238434 --- /dev/null +++ b/e2e/fixtures/media/not-an-image.jpg @@ -0,0 +1,4 @@ +This is plain text, not an image at all. +This is plain text, not an image at all. +This is plain text, not an image at all. +This is plain text, not an image at all. diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index bdf73ad..175272e 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -27,6 +27,13 @@ async function request(method: string, path: string, body?: unknown): Promise // Abort hung requests so a dead connection surfaces as a friendly error // instead of a spinner that never resolves. + // + // The timer must stay armed until the BODY has been read, not just the headers. + // `fetch` resolves as soon as the response head arrives, so clearing it in a `finally` + // around the fetch left `res.text()` below completely uncovered — and no longer + // abortable, since the controller had already been disarmed. An upstream that sends + // headers and then stalls the body (the shape of a half-dead proxy, or of the pool + // saturation this same release adds shedding for) hung that call forever. const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), TIMEOUT_MS); @@ -38,6 +45,26 @@ async function request(method: string, path: string, body?: unknown): Promise body: body !== undefined ? JSON.stringify(body) : undefined, signal: controller.signal }); + } catch (e) { + clearTimeout(timer); + if (e instanceof DOMException && e.name === 'AbortError') { + throw new ApiError(0, 'timeout', 'Zeitüberschreitung – bitte erneut versuchen.'); + } + throw new ApiError(0, 'network', 'Netzwerkfehler – bitte Verbindung prüfen.'); + } + + if (res.status === 204) { + // Must clear on this path too, or every no-content request (logout, delete, like) + // leaks a live 20 s timer. + clearTimeout(timer); + return undefined as T; + } + + // A 5xx behind a proxy (or a crash page) can return HTML, not JSON — parsing + // it directly would throw an opaque SyntaxError. Read text, parse defensively. + let raw: string; + try { + raw = await res.text(); } catch (e) { if (e instanceof DOMException && e.name === 'AbortError') { throw new ApiError(0, 'timeout', 'Zeitüberschreitung – bitte erneut versuchen.'); @@ -47,13 +74,6 @@ async function request(method: string, path: string, body?: unknown): Promise clearTimeout(timer); } - if (res.status === 204) { - return undefined as T; - } - - // A 5xx behind a proxy (or a crash page) can return HTML, not JSON — parsing - // it directly would throw an opaque SyntaxError. Read text, parse defensively. - const raw = await res.text(); let data: { error?: string; message?: string } | unknown = null; if (raw) { try { diff --git a/frontend/src/lib/upload-queue.test.ts b/frontend/src/lib/upload-queue.test.ts index e861cde..b5b8072 100644 --- a/frontend/src/lib/upload-queue.test.ts +++ b/frontend/src/lib/upload-queue.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from 'vitest'; -import { classifyUploadStatus, isReversibleLock, entryToQueueItem } from './upload-queue'; +import { + classifyUploadStatus, + isReversibleLock, + entryToQueueItem, + shouldAbortForStall +} from './upload-queue'; /** * Regression guard for the upload-queue retry policy (H2 + M1). The bug being locked out: @@ -107,3 +112,32 @@ describe('entryToQueueItem', () => { expect(item.hashtags).toBe(''); }); }); + +/** + * The upload XHR had no timeout of any kind while `processQueue` held the `isProcessing` + * latch across it. On a half-open socket neither `error` nor `abort` ever fires, so the + * latch was pinned forever and the whole queue wedged with no recovery but a reload. + * + * The policy that matters: bound SILENCE, not total duration. A 500 MB video over a venue + * uplink legitimately runs 30+ minutes while making steady progress, and a flat total cap + * would kill exactly the uploads worth keeping. + */ +describe('shouldAbortForStall', () => { + const now = 1_000_000; + + it('lets a long upload run as long as progress keeps arriving', () => { + // Two hours in, but progress landed a second ago. + expect(shouldAbortForStall(now - 1_000, now, false)).toBe(false); + }); + + it('aborts once the body stalls past the no-progress ceiling', () => { + expect(shouldAbortForStall(now - 89_000, now, false)).toBe(false); + expect(shouldAbortForStall(now - 91_000, now, false)).toBe(true); + }); + + it('applies the wider ceiling once the body is sent and progress goes quiet', () => { + // Silence that would abort mid-body is normal while waiting for the response. + expect(shouldAbortForStall(now - 91_000, now, true)).toBe(false); + expect(shouldAbortForStall(now - 121_000, now, true)).toBe(true); + }); +}); diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index 74ba9e5..06d396c 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -37,6 +37,37 @@ const STORE_NAME = 'queue'; /** Hard cap on queued items per device — bounds IndexedDB growth from stuck blobs. */ const MAX_QUEUE_ITEMS = 100; +// ── Upload watchdog ─────────────────────────────────────────────────────────── +// The upload XHR had no timeout of any kind while `processQueue` held the `processing` / +// `isProcessing` latch across it. A half-open socket — the classic phone-leaves-wifi case, +// where no `error` and no `abort` event ever fires — pinned that latch forever and wedged +// the whole queue with no recovery short of a reload. +// +// Deliberately NOT a flat total timeout: a legitimate 500 MB video over a venue uplink can +// run 30+ minutes while making perfectly steady progress, and a total cap would kill exactly +// the uploads that matter most. What we bound is SILENCE. + +/** No-progress ceiling while the request body is still being sent. */ +const UPLOAD_STALL_MS = 90_000; +/** Ceiling for the window AFTER the last byte is sent. `upload.progress` is silent there by + * definition (the server is sniffing, committing and answering), so the stall detector has + * no signal and this coarser bound takes over. Sized above the backend's own worst-case + * commit path, not above compression — compression is async and does not hold the response. */ +const UPLOAD_RESPONSE_TIMEOUT_MS = 120_000; +/** How often the watchdog re-checks. Coarse on purpose; it only needs to bound the wedge. */ +const UPLOAD_WATCHDOG_INTERVAL_MS = 5_000; + +/** Pure predicate behind the watchdog, extracted so the policy is unit-testable without + * standing up an XHR harness. */ +export function shouldAbortForStall( + lastActivityAt: number, + now: number, + bodySent: boolean +): boolean { + const ceiling = bodySent ? UPLOAD_RESPONSE_TIMEOUT_MS : UPLOAD_STALL_MS; + return now - lastActivityAt > ceiling; +} + let db: IDBPDatabase | null = null; // Resume the queue as soon as connectivity returns. Registered once, guarded for SSR. @@ -139,8 +170,10 @@ async function getDb(): Promise { * blamed for) the previous guest's pending uploads. */ export async function clearQueue(): Promise { - const database = await getDb(); - await database.clear(STORE_NAME); + // Always clear the in-memory view, even if the store is unreachable: this runs on logout, + // and leaving the previous guest's items on screen for the next one is the worse failure. + const database = await getDbSafe(); + if (database) await database.clear(STORE_NAME); queueItems.set([]); rateLimitRetryAt.set(null); } @@ -287,15 +320,27 @@ export async function loadQueue(): Promise { } /** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT - * actually queued (deduped, or the queue is full of un-evictable in-flight items). */ -export type EnqueueResult = 'queued' | 'duplicate' | 'full'; + * actually queued (deduped, the queue is full of un-evictable in-flight items, or the + * local store itself is unusable). */ +export type EnqueueResult = 'queued' | 'duplicate' | 'full' | 'failed'; export async function addToQueue( file: File, caption: string, hashtags: string ): Promise { - const database = await getDb(); + // IndexedDB is not guaranteed available: Safari private mode refuses to open a DB, an + // upgrade can be blocked by another tab, and `put` of a 500 MB blob can hit a + // QuotaExceededError. Every one of those used to reject out of here into a `handleSubmit` + // with no catch — leaving a permanent "Wird hochgeladen…" spinner, no toast, and (for an + // in-app camera capture) the only copy of the photo gone. Report it instead. + let database: IDBPDatabase; + try { + database = await getDb(); + } catch (e) { + console.warn('upload queue unavailable (IndexedDB)', e); + return 'failed'; + } const userId = getUserId(); // Not authenticated — nothing to queue. Return the silent 'duplicate' outcome rather than // 'full' so the caller doesn't show a misleading "queue full" toast. Practically @@ -348,7 +393,14 @@ export async function addToQueue( status: 'pending', blob: file }; - await database.put(STORE_NAME, entry); + // Persist BEFORE touching the store: a failure here (quota exceeded on a large blob) must + // not leave a phantom item the UI shows as queued but nothing can ever upload. + try { + await database.put(STORE_NAME, entry); + } catch (e) { + console.warn('upload queue write failed (IndexedDB)', e); + return 'failed'; + } queueItems.update((items) => [ ...items, @@ -370,8 +422,21 @@ export async function addToQueue( return 'queued'; } +/** `getDb` that reports unavailability instead of rejecting. For the UI-driven queue + * operations, where an unusable IndexedDB must degrade to "nothing happened" rather than + * leave a caller awaiting a rejected promise it never catches. */ +async function getDbSafe(): Promise { + try { + return await getDb(); + } catch (e) { + console.warn('upload queue unavailable (IndexedDB)', e); + return null; + } +} + export async function retryItem(id: string): Promise { - const database = await getDb(); + const database = await getDbSafe(); + if (!database) return; const entry = await database.get(STORE_NAME, id); if (!entry) return; @@ -389,13 +454,15 @@ export async function retryItem(id: string): Promise { } export async function removeItem(id: string): Promise { - const database = await getDb(); + const database = await getDbSafe(); + if (!database) return; await database.delete(STORE_NAME, id); queueItems.update((items) => items.filter((item) => item.id !== id)); } export async function clearCompleted(): Promise { - const database = await getDb(); + const database = await getDbSafe(); + if (!database) return; const items = get(queueItems); for (const item of items) { if (item.status === 'done') { @@ -495,7 +562,24 @@ async function uploadItem(id: string): Promise { xhr.open('POST', '/api/v1/upload'); xhr.setRequestHeader('Authorization', `Bearer ${token}`); + // Watchdog state — see UPLOAD_STALL_MS. `timedOut` distinguishes our own abort from + // a user-initiated one so the surfaced message stays honest. + let lastActivityAt = Date.now(); + let bodySent = false; + let timedOut = false; + let watchdog: ReturnType | undefined; + const stopWatchdog = () => { + if (watchdog !== undefined) { + clearInterval(watchdog); + watchdog = undefined; + } + }; + // `loadend` on the XHR itself fires on success, error, abort and timeout alike — the + // one hook that guarantees the interval is released on every exit path. + xhr.addEventListener('loadend', stopWatchdog); + xhr.upload.addEventListener('progress', (e) => { + lastActivityAt = Date.now(); if (e.lengthComputable) { const pct = Math.round((e.loaded / e.total) * 100); queueItems.update((items) => @@ -557,9 +641,27 @@ async function uploadItem(id: string): Promise { } }); + // Body fully handed to the socket: `upload.progress` goes quiet from here, so switch + // the watchdog to the response ceiling rather than let it fire on normal waiting. + xhr.upload.addEventListener('loadend', () => { + bodySent = true; + lastActivityAt = Date.now(); + }); + xhr.addEventListener('error', () => reject(new NetworkError('Netzwerkfehler'))); - xhr.addEventListener('abort', () => reject(new NetworkError('Abgebrochen'))); + xhr.addEventListener('abort', () => + reject(new NetworkError(timedOut ? 'Zeitüberschreitung' : 'Abgebrochen')) + ); xhr.send(formData); + // Armed only after send(), so the clock starts with the request. NetworkError is + // already the retryable branch (blob kept, "Erneut" offered), so a stalled upload + // now recovers exactly like a network blip instead of wedging the queue. + watchdog = setInterval(() => { + if (!shouldAbortForStall(lastActivityAt, Date.now(), bodySent)) return; + stopWatchdog(); + timedOut = true; + xhr.abort(); + }, UPLOAD_WATCHDOG_INTERVAL_MS); }); // Success — remove blob from IndexedDB, mark done diff --git a/frontend/src/routes/upload/+page.svelte b/frontend/src/routes/upload/+page.svelte index 9c7eae5..ed72d97 100644 --- a/frontend/src/routes/upload/+page.svelte +++ b/frontend/src/routes/upload/+page.svelte @@ -105,9 +105,24 @@ vibrate(10); const hashtagsString = captionTags.join(','); let full = 0; - for (const sf of stagedFiles) { - const result = await addToQueue(sf.file, caption, hashtagsString); - if (result === 'full') full++; + let failed = 0; + try { + for (const sf of stagedFiles) { + const result = await addToQueue(sf.file, caption, hashtagsString); + if (result === 'full') full++; + else if (result === 'failed') failed++; + } + } catch (e) { + // `addToQueue` reports its own storage failures as 'failed', so reaching here means + // something genuinely unexpected. Treat the whole batch as not-queued rather than + // navigating away from photos that were never persisted. + console.error('staging failed', e); + failed = stagedFiles.length; + } finally { + // Must be released on EVERY path. Without this the submit button — disabled on + // `submitting` — stayed stuck on "Wird hochgeladen…" forever after any throw, with + // no error shown and no way forward. + submitting = false; } // Don't let a full queue silently swallow photos the user thinks were queued. if (full > 0) { @@ -117,6 +132,18 @@ 6000 ); } + // Local storage is unusable (private mode, blocked upgrade, quota). Keep the staged + // files ON SCREEN and stay on the page — navigating away would destroy in-app camera + // captures that exist nowhere else. Re-submitting is safe and is not a double-queue: + // `addToQueue` dedups on name+size+lastModified+userId while an item is pending. + if (failed > 0) { + toast( + `${failed} ${failed === 1 ? 'Foto konnte' : 'Fotos konnten'} nicht gespeichert werden. Bitte Speicherplatz prüfen und erneut versuchen.`, + 'error', + 6000 + ); + return; + } clearPending(); goto('/feed'); }