From cef41ce76a0cfa0f71e031cd13b26d6929781081 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 11 Jul 2026 13:55:50 +0200 Subject: [PATCH] fix: don't burn a job attempt when the browser is unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A job's attempt is incremented at lease time, then the dispatcher acquires the headless browser. When the browser was down or mid-restart, acquire() failed and the error was ack_failed — so during an outage the whole pending backlog was chewed to `dead` within minutes while workers hot-looped. Add SyncOutcome::BrowserUnavailable: the dispatcher returns it instead of propagating the acquire error, and the daemon releases the lease (refunding the attempt, like the cancel/session paths) and backs off, so an outage is treated as an infrastructure blip rather than per-job failures. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/app.rs | 22 ++++++++++- backend/src/bin/crawler.rs | 7 ++++ backend/src/crawler/content.rs | 7 ++++ backend/src/crawler/daemon.rs | 24 ++++++++++++ backend/src/crawler/resync.rs | 6 +++ backend/tests/crawler_daemon.rs | 66 +++++++++++++++++++++++++++++++++ frontend/package.json | 2 +- 9 files changed, 133 insertions(+), 5 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index b902bd0..adb6422 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.124.16" +version = "0.124.17" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 8c286bc..67632ec 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.124.16" +version = "0.124.17" edition = "2021" default-run = "mangalord" diff --git a/backend/src/app.rs b/backend/src/app.rs index f3831aa..574176c 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -990,7 +990,17 @@ impl ChapterDispatcher for RealChapterDispatcher { pages_done: 0, pages_total: None, }); - let lease = self.browser_manager.acquire().await?; + let lease = match self.browser_manager.acquire().await { + Ok(l) => l, + Err(e) => { + // Browser down / mid-restart: defer the job WITHOUT + // burning an attempt (the daemon releases it back to + // pending) rather than counting an infrastructure + // outage as a per-job failure. + tracing::warn!(error = ?e, "dispatch: browser unavailable — deferring job"); + return Ok(SyncOutcome::BrowserUnavailable); + } + }; let result = content::sync_chapter_content( &lease, &self.db, @@ -1061,7 +1071,15 @@ impl ChapterDispatcher for RealChapterDispatcher { // Scope the lease so it (and the borrowing FetchContext) drop // before any browser-restart handling in the match below. let result = { - let lease = self.browser_manager.acquire().await?; + let lease = match self.browser_manager.acquire().await { + Ok(l) => l, + Err(e) => { + // See the SyncChapterContent arm: defer without + // burning an attempt when the browser is unavailable. + tracing::warn!(error = ?e, "dispatch: browser unavailable — deferring job"); + return Ok(SyncOutcome::BrowserUnavailable); + } + }; let ctx = crate::crawler::source::FetchContext { browser: &lease, rate: &self.rate, diff --git a/backend/src/bin/crawler.rs b/backend/src/bin/crawler.rs index 72949c5..e15cd6e 100644 --- a/backend/src/bin/crawler.rs +++ b/backend/src/bin/crawler.rs @@ -441,6 +441,13 @@ async fn sync_bookmarked_chapter_content( s.fetched += 1; } Ok(SyncOutcome::Skipped) => s.skipped += 1, + // Unreachable in the one-shot CLI (it holds its own lease + // and never dispatches through the queue), but count it as a + // failure for exhaustiveness. + Ok(SyncOutcome::BrowserUnavailable) => { + tracing::warn!(%chapter_id, "crawler browser unavailable"); + s.failed += 1; + } Ok(SyncOutcome::SessionExpired) => { tracing::error!( %chapter_id, diff --git a/backend/src/crawler/content.rs b/backend/src/crawler/content.rs index d813a9f..a4b3828 100644 --- a/backend/src/crawler/content.rs +++ b/backend/src/crawler/content.rs @@ -71,6 +71,13 @@ pub enum SyncOutcome { /// Session probe failed mid-sync (avatar selector missing on the /// chapter page). Caller should abort the whole crawler run. SessionExpired, + /// The headless browser could not be acquired (down or mid-restart). + /// Produced only by the queue dispatcher (which calls `acquire()`); it is + /// an *infrastructure* outage, not a job failure, so the daemon returns the + /// job to `pending` WITHOUT burning a retry attempt. `sync_chapter_content` + /// itself never returns this — callers that already hold a lease can treat + /// it as unreachable. + BrowserUnavailable, } /// Per-chapter max fetch attempts when TOR is configured. `N = 3` means diff --git a/backend/src/crawler/daemon.rs b/backend/src/crawler/daemon.rs index 4aee574..9c23d5a 100644 --- a/backend/src/crawler/daemon.rs +++ b/backend/src/crawler/daemon.rs @@ -66,6 +66,12 @@ const LEASE_DURATION: Duration = Duration::from_secs(60); /// the lease window leaves two missed-beat's slack before expiry. const LEASE_HEARTBEAT: Duration = Duration::from_secs(20); +/// How long a worker waits after a `BrowserUnavailable` outcome before looping +/// back to lease again. The job was released (not failed) so it stays pending; +/// this backoff keeps the worker from hot-looping lease→acquire→release while +/// the browser is down or mid-restart. +const BROWSER_UNAVAILABLE_BACKOFF: Duration = Duration::from_secs(5); + #[async_trait] pub trait MetadataPass: Send + Sync { async fn run(&self) -> anyhow::Result; @@ -502,6 +508,24 @@ impl WorkerContext { self.status.poke(); let _ = jobs::release(&self.pool, lease.id, lease.lease_generation).await; } + Ok(Ok(SyncOutcome::BrowserUnavailable)) => { + // Infrastructure outage, not a job failure: the browser was + // down or mid-restart when the dispatcher tried to acquire it. + // Return the job to `pending` WITHOUT burning an attempt (like + // the cancel/session paths) so an outage doesn't chew the whole + // backlog to `dead`, then back off to avoid hot-looping while + // the browser recovers. + tracing::warn!( + worker = self.id, + lease_id = %lease.id, + "worker: browser unavailable — released lease without burning an attempt" + ); + let _ = jobs::release(&self.pool, lease.id, lease.lease_generation).await; + tokio::select! { + _ = tokio::time::sleep(BROWSER_UNAVAILABLE_BACKOFF) => {} + _ = self.cancel.cancelled() => {} + } + } Ok(Err(e)) => { tracing::warn!( worker = self.id, diff --git a/backend/src/crawler/resync.rs b/backend/src/crawler/resync.rs index e9334b5..25ce459 100644 --- a/backend/src/crawler/resync.rs +++ b/backend/src/crawler/resync.rs @@ -280,6 +280,12 @@ impl ResyncService for RealResyncService { SyncOutcome::SessionExpired => { anyhow::bail!("source session expired — operator must refresh PHPSESSID") } + // Unreachable here: resync already holds its own browser lease and + // `sync_chapter_content` never acquires one, so it can't report the + // browser unavailable. Handled defensively for exhaustiveness. + SyncOutcome::BrowserUnavailable => { + anyhow::bail!("crawler browser unavailable") + } } } } diff --git a/backend/tests/crawler_daemon.rs b/backend/tests/crawler_daemon.rs index 4dad184..d7e1950 100644 --- a/backend/tests/crawler_daemon.rs +++ b/backend/tests/crawler_daemon.rs @@ -104,6 +104,20 @@ impl ChapterDispatcher for FailingDispatcher { } } +/// Always reports the browser unavailable — models a Chromium outage where +/// `acquire()` fails. The job must be returned to `pending` WITHOUT burning an +/// attempt, so an outage doesn't chew the backlog to `dead`. +struct BrowserUnavailableDispatcher { + seen: AtomicUsize, +} +#[async_trait::async_trait] +impl ChapterDispatcher for BrowserUnavailableDispatcher { + async fn dispatch(&self, _payload: JobPayload) -> anyhow::Result { + self.seen.fetch_add(1, Ordering::AcqRel); + Ok(SyncOutcome::BrowserUnavailable) + } +} + /// Never completes — used to verify the worker's outer dispatch timeout. struct HangingDispatcher { seen: AtomicUsize, @@ -204,6 +218,58 @@ async fn shutdown_mid_dispatch_releases_lease_without_burning_attempt(pool: PgPo ); } +#[sqlx::test(migrations = "./migrations")] +async fn browser_unavailable_releases_lease_without_burning_attempt(pool: PgPool) { + // During a browser outage the dispatcher reports BrowserUnavailable. The + // worker must return the job to `pending` with the attempt refunded + // (attempts stays 0) instead of ack-failing it toward `dead`, so the whole + // pending backlog survives the outage. + enqueue_chapter_job(&pool).await; + let dispatcher = Arc::new(BrowserUnavailableDispatcher { + seen: AtomicUsize::new(0), + }); + let session_expired = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let cancel = CancellationToken::new(); + let handle = daemon::spawn( + pool.clone(), + cancel.clone(), + make_cfg(None, dispatcher.clone(), session_expired, 1), + ); + + // Wait until the dispatcher has been invoked at least once (the job was + // leased and deferred). + let mut dispatched = false; + for _ in 0..40 { + if dispatcher.seen.load(Ordering::Acquire) >= 1 { + dispatched = true; + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert!(dispatched, "dispatcher must have been invoked"); + + handle.shutdown().await; + + assert_eq!( + count_state(&pool, "dead").await, + 0, + "browser outage must not dead-letter the job" + ); + assert_eq!( + count_state(&pool, "pending").await, + 1, + "job returns to pending after a browser-unavailable outcome" + ); + let attempts: i32 = sqlx::query_scalar("SELECT attempts FROM crawler_jobs") + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!( + attempts, 0, + "browser-unavailable must refund the lease attempt (no burn)" + ); +} + #[sqlx::test(migrations = "./migrations")] async fn workers_drain_jobs_through_dispatcher(pool: PgPool) { enqueue_chapter_job(&pool).await; diff --git a/frontend/package.json b/frontend/package.json index 99db7dd..2a0c67a 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.124.16", + "version": "0.124.17", "private": true, "type": "module", "scripts": {