From 59d0f4bec83197b890d3e05ab84f0da3ac059cdf Mon Sep 17 00:00:00 2001 From: orange-bot Date: Thu, 8 Oct 2026 12:59:16 +0800 Subject: [PATCH] fix(catalog): stop failing slow Mod catalog downloads after 8s The 8s wait in fetch_catalog_interruptible treated a merely slow connection as an offline server: - the response that was still on its way was dropped, even though the HTTP request itself may run for its own 20s timeout - load_catalog latched CATALOG_OFFLINE on that timeout, so every later catalog read served the cache (or failed) until the user manually pressed "reconnect" Keep the 8s deadline as "do not block the first screen any longer" instead of a failure: - wait_for_catalog now reports Finished/StillDownloading/Skipped - a download that outlives the wait keeps running and publishes its response to the on-disk cache when it lands (unless the request was cancelled, in which case nothing is written) - load_catalog only latches offline mode for real failures, and serves the cached catalog meanwhile - the catalog request timeout is 60s: the Mod list decompresses to ~5.8MB (6265 Mods), so 20s required ~290KB/s and 8s ~700KB/s - new is_mod_catalog_downloading command lets the UI wait for a running download; loadModCatalog retries once after it settles and the startup flow refreshes the Mod list, so slow users end up with the metadata instead of an empty list --- src-tauri/src/backend.rs | 6 + src-tauri/src/everest.rs | 200 +++++++++++++++++---- src/celemod-ui/package.json | 1 + src/celemod-ui/src/api/modCatalog.ts | 59 ++++-- src/celemod-ui/src/catalogDownload.test.ts | 43 +++++ src/celemod-ui/src/catalogDownload.ts | 26 +++ src/celemod-ui/src/context/modManage.tsx | 16 +- src/celemod-ui/src/tauri/commands.ts | 1 + 8 files changed, 302 insertions(+), 50 deletions(-) create mode 100644 src/celemod-ui/src/catalogDownload.test.ts create mode 100644 src/celemod-ui/src/catalogDownload.ts diff --git a/src-tauri/src/backend.rs b/src-tauri/src/backend.rs index 3864059..ae8327a 100644 --- a/src-tauri/src/backend.rs +++ b/src-tauri/src/backend.rs @@ -4704,6 +4704,11 @@ fn is_catalog_offline() -> bool { everest::is_catalog_offline() } +#[tauri::command] +fn is_mod_catalog_downloading() -> bool { + everest::is_catalog_downloading() +} + #[tauri::command] fn is_using_cache() -> bool { everest::is_using_cache() @@ -6391,6 +6396,7 @@ pub fn run() { write_frontend_log, set_catalog_offline, is_catalog_offline, + is_mod_catalog_downloading, is_using_cache, configure_mod_cache, get_mod_catalog, diff --git a/src-tauri/src/everest.rs b/src-tauri/src/everest.rs index a9a7ca4..69cfa28 100644 --- a/src-tauri/src/everest.rs +++ b/src-tauri/src/everest.rs @@ -28,6 +28,24 @@ pub struct ModInfoCached { static CATALOG_OFFLINE: AtomicBool = AtomicBool::new(false); static CATALOG_REQUEST_GENERATION: AtomicU64 = AtomicU64::new(0); +// Downloads that are still running because a caller stopped waiting for them +// (see CATALOG_SOFT_TIMEOUT). The UI polls this to know that a fresh catalog is +// on its way, so a slow connection is not mistaken for an offline one. +static CATALOG_DOWNLOADS_IN_FLIGHT: AtomicU64 = AtomicU64::new(0); + +/// How long the catalog download itself may take. This is only a bound for a +/// server that stops sending: the first screen no longer blocks on the download +/// (see [`CATALOG_SOFT_TIMEOUT`]), so the request may be given the room a slow +/// connection needs instead of being cut short. +const CATALOG_FETCH_TIMEOUT: Duration = Duration::from_secs(60); +/// How long a caller waits for the catalog before continuing with whatever is +/// already cached. The download keeps running in the background and publishes +/// its response when it lands, so exceeding this is a slow connection rather +/// than an offline server. +const CATALOG_SOFT_TIMEOUT: Duration = Duration::from_secs(8); +/// Polling granularity of `wait_for_catalog`: the worker is checked at least +/// this often so a cancellation is noticed quickly. +const CATALOG_POLL_INTERVAL: Duration = Duration::from_millis(100); pub fn set_catalog_offline(offline: bool) { CATALOG_OFFLINE.store(offline, Ordering::SeqCst); @@ -38,6 +56,10 @@ pub fn is_catalog_offline() -> bool { CATALOG_OFFLINE.load(Ordering::SeqCst) } +pub fn is_catalog_downloading() -> bool { + CATALOG_DOWNLOADS_IN_FLIGHT.load(Ordering::SeqCst) > 0 +} + static USING_CACHE: AtomicBool = AtomicBool::new(false); static MOD_CACHE_TTL_SECONDS: AtomicU64 = AtomicU64::new(60 * 60); @@ -121,43 +143,115 @@ fn fetch_raw_catalog() -> anyhow::Result { "User-Agent", &format!("CeleMod/{}-{}", env!("VERSION"), &env!("GIT_HASH")[..6]), ) - .timeout(std::time::Duration::from_secs(20)) + .timeout(CATALOG_FETCH_TIMEOUT) .set("Accept-Encoding", "gzip, deflate, br") .call()? .into_string()?) } -// Only the raw response crosses the worker boundary: a skipped request must -// never publish a late response or overwrite the cache. The HTTP worker itself -// is bounded by fetch_raw_catalog's timeout. +/// How a wait for the catalog worker ended. +enum CatalogWait { + /// The worker answered, successfully or not. + Finished(anyhow::Result), + /// The caller ran out of patience while the worker keeps downloading. This + /// is not a failure: the response is still on its way. + StillDownloading, + /// The request was cancelled: offline mode was enabled or a newer request + /// superseded this one. + Skipped, +} + +/// The catalog download is still running even though the caller stopped waiting +/// for it. Unlike a network failure this must not switch the app to offline +/// mode, because the response is on its way and will be published as soon as it +/// lands. +#[derive(Debug)] +struct CatalogDownloadPending; + +impl std::fmt::Display for CatalogDownloadPending { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + formatter, + "Mod catalog is still downloading; continuing with the cached catalog" + ) + } +} + +impl std::error::Error for CatalogDownloadPending {} + +/// A catalog fetch that only ran out of time is not evidence of an unreachable +/// server, so it must not latch offline mode for the rest of the session. +fn is_catalog_download_pending(error: &anyhow::Error) -> bool { + error.downcast_ref::().is_some() +} + +/// Whether a finished response still belongs to the request the app wants. +fn request_is_still_wanted(offline: bool, current_generation: u64, generation: u64) -> bool { + !offline && current_generation == generation +} + +/// A skipped request must never overwrite the cache with a late response or +/// latch offline mode for it. +fn catalog_response_is_still_wanted(generation: u64) -> bool { + request_is_still_wanted( + is_catalog_offline(), + CATALOG_REQUEST_GENERATION.load(Ordering::SeqCst), + generation, + ) +} + +// The worker outlives the caller: a slow connection is not a reason to throw a +// response away, but a cancelled request must still publish nothing. fn fetch_catalog_interruptible() -> anyhow::Result { let generation = CATALOG_REQUEST_GENERATION.load(Ordering::SeqCst); let (sender, receiver) = std::sync::mpsc::channel(); + CATALOG_DOWNLOADS_IN_FLIGHT.fetch_add(1, Ordering::SeqCst); std::thread::spawn(move || { - let _ = sender.send(fetch_raw_catalog()); + let result = fetch_raw_catalog(); + let still_wanted = catalog_response_is_still_wanted(generation); + match &result { + // Nobody may be waiting for a slow connection anymore: keep the + // response in the cache so the next catalog read (or the frontend + // retry) picks it up instead of giving up on it. + Ok(raw) if still_wanted => save_raw_cache(raw), + // Remember an unreachable server even when the caller already moved + // on, so local operations stop retrying it in a loop. + Err(_) if still_wanted => CATALOG_OFFLINE.store(true, Ordering::SeqCst), + _ => {} + } + CATALOG_DOWNLOADS_IN_FLIGHT.fetch_sub(1, Ordering::SeqCst); + let _ = sender.send(result); }); - wait_for_catalog(receiver, Duration::from_secs(8), || { + match wait_for_catalog(receiver, CATALOG_SOFT_TIMEOUT, || { is_catalog_offline() || CATALOG_REQUEST_GENERATION.load(Ordering::SeqCst) != generation - }) + }) { + CatalogWait::Finished(result) => result, + CatalogWait::StillDownloading => Err(CatalogDownloadPending.into()), + CatalogWait::Skipped => bail!("Mod catalog download skipped"), + } } fn wait_for_catalog( receiver: std::sync::mpsc::Receiver>, timeout: Duration, cancelled: impl Fn() -> bool, -) -> anyhow::Result { +) -> CatalogWait { let started = std::time::Instant::now(); loop { if started.elapsed() >= timeout { - bail!("Mod catalog download timed out; continuing offline"); + return CatalogWait::StillDownloading; } if cancelled() { - bail!("Mod catalog download skipped"); + return CatalogWait::Skipped; } - match receiver.recv_timeout(Duration::from_millis(100)) { - Ok(result) => return result, + match receiver.recv_timeout(CATALOG_POLL_INTERVAL) { + Ok(result) => return CatalogWait::Finished(result), Err(std::sync::mpsc::RecvTimeoutError::Timeout) => continue, - Err(error) => return Err(error.into()), + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { + return CatalogWait::Finished(Err(anyhow::anyhow!( + "Mod catalog worker stopped without a response" + ))); + } } } } @@ -258,6 +352,12 @@ fn load_catalog(force_refresh: bool) -> anyhow::Result { Ok(state) } Err(network_error) => { + // A download that is still running is not an offline server: serve + // the cached catalog, keep offline mode off, and let the background + // download publish the fresh catalog once it lands. + if is_catalog_download_pending(&network_error) { + return offline_catalog().map_err(|_| network_error); + } crate::logging::error(format_args!( "Failed to fetch Mod catalog: {network_error:#}" )); @@ -599,7 +699,7 @@ pub fn download_and_install_everest( #[cfg(test)] mod tests { - use super::{is_everest_ultra, wait_for_catalog}; + use super::{CatalogWait, is_everest_ultra, wait_for_catalog}; use std::time::Duration; use std::time::{SystemTime, UNIX_EPOCH}; @@ -623,48 +723,72 @@ mod tests { fn catalog_wait_returns_success_and_network_errors() { let (sender, receiver) = std::sync::mpsc::channel(); sender.send(Ok("catalog".into())).unwrap(); - assert_eq!( - wait_for_catalog(receiver, Duration::from_secs(1), || false).unwrap(), - "catalog" - ); + match wait_for_catalog(receiver, Duration::from_secs(1), || false) { + CatalogWait::Finished(Ok(raw)) => assert_eq!(raw, "catalog"), + _ => panic!("a received response must finish the wait"), + } let (sender, receiver) = std::sync::mpsc::channel(); sender.send(Err(anyhow::anyhow!("no network"))).unwrap(); - assert!( - wait_for_catalog(receiver, Duration::from_secs(1), || false) - .unwrap_err() - .to_string() - .contains("no network") - ); + match wait_for_catalog(receiver, Duration::from_secs(1), || false) { + CatalogWait::Finished(Err(error)) => { + assert!(error.to_string().contains("no network")) + } + _ => panic!("a worker error must finish the wait"), + } } #[test] fn catalog_skip_discards_even_an_already_received_response() { let (sender, receiver) = std::sync::mpsc::channel(); sender.send(Ok("late catalog".into())).unwrap(); - assert!( - wait_for_catalog(receiver, Duration::from_secs(1), || true) - .unwrap_err() - .to_string() - .contains("skipped") - ); + assert!(matches!( + wait_for_catalog(receiver, Duration::from_secs(1), || true), + CatalogWait::Skipped + )); } #[test] - fn catalog_wait_is_bounded_without_network_response() { + fn slow_catalog_download_keeps_the_wait_open_instead_of_failing() { + // A connection that is merely slow must not be reported as an error: + // the download keeps running and publishes the catalog when it lands. let (_sender, receiver) = std::sync::mpsc::channel(); - assert!( - wait_for_catalog(receiver, Duration::from_millis(1), || false) - .unwrap_err() - .to_string() - .contains("timed out") - ); + assert!(matches!( + wait_for_catalog(receiver, Duration::from_millis(1), || false), + CatalogWait::StillDownloading + )); } #[test] fn catalog_wait_handles_worker_failure() { let (sender, receiver) = std::sync::mpsc::channel(); drop(sender); - assert!(wait_for_catalog(receiver, Duration::from_secs(1), || false).is_err()); + assert!(matches!( + wait_for_catalog(receiver, Duration::from_secs(1), || false), + CatalogWait::Finished(Err(_)) + )); + } + + #[test] + fn only_real_catalog_failures_switch_to_offline_mode() { + // The regression behind the slow-network bug: running out of the 8s + // wait used to latch offline mode and drop the response that was still + // on its way. + assert!(super::is_catalog_download_pending( + &super::CatalogDownloadPending.into() + )); + assert!(!super::is_catalog_download_pending(&anyhow::anyhow!( + "no network" + ))); + } + + #[test] + fn late_catalog_responses_are_only_published_while_still_wanted() { + // A response that lands after the caller gave up is still published, so + // a slow connection ends up with a fresh catalog. A request that was + // cancelled (offline mode or a newer request) must be dropped instead. + assert!(super::request_is_still_wanted(false, 7, 7)); + assert!(!super::request_is_still_wanted(true, 7, 7)); + assert!(!super::request_is_still_wanted(false, 8, 7)); } #[test] diff --git a/src/celemod-ui/package.json b/src/celemod-ui/package.json index a4576a1..5432a7e 100644 --- a/src/celemod-ui/package.json +++ b/src/celemod-ui/package.json @@ -13,6 +13,7 @@ "lint": "tsc --noEmit --noUnusedLocals --noUnusedParameters", "translate": "di18n sync", "test:i18n": "node --test scripts/deepseek-translator.test.cjs", + "test:catalog-download": "node --import tsx --test src/catalogDownload.test.ts", "test:crash-trace": "node --import tsx --test src/components/crashTrace.test.ts", "test:deep-link": "node --import tsx --test src/deepLink.test.ts", "test:update-notes": "node --import tsx --test src/components/UpdateNotes.test.tsx" diff --git a/src/celemod-ui/src/api/modCatalog.ts b/src/celemod-ui/src/api/modCatalog.ts index 0052da0..1b87feb 100644 --- a/src/celemod-ui/src/api/modCatalog.ts +++ b/src/celemod-ui/src/api/modCatalog.ts @@ -1,5 +1,6 @@ import type { Content, File as SubmissionFile } from "./wegfan"; -import { callRemote } from "../utils"; +import { callRemote, sleep } from "../utils"; +import { waitForCatalogDownload } from "../catalogDownload"; export interface CatalogSubmission { id: string; @@ -81,6 +82,28 @@ let catalog: CatalogMod[] | null = null; let catalogPromise: Promise | null = null; let catalogRequestId = 0; +/** + * Whether the native side is still downloading the Mod catalog. It keeps a slow + * download running after the first screen stopped waiting for it, so the UI can + * wait for it to land instead of showing an empty catalog. + */ +export const isCatalogDownloading = async (): Promise => { + try { + return await callRemote("is_mod_catalog_downloading"); + } catch (error) { + console.warn("Failed to query the Mod catalog download state", error); + return false; + } +}; + +const requestModCatalog = (requestId: number, forceRefresh: boolean) => + callRemote("get_mod_catalog", forceRefresh).then((raw) => { + const parsed = JSON.parse(raw) as CatalogResponse; + const nextCatalog = Array.isArray(parsed.data) ? parsed.data : []; + if (requestId === catalogRequestId) catalog = nextCatalog; + return nextCatalog; + }); + export const loadModCatalog = async ( ttlHours = 1, forceRefresh = false, @@ -91,16 +114,30 @@ export const loadModCatalog = async ( const requestId = ++catalogRequestId; let request: Promise; - request = callRemote("get_mod_catalog", forceRefresh) - .then((raw) => { - const parsed = JSON.parse(raw) as CatalogResponse; - const nextCatalog = Array.isArray(parsed.data) ? parsed.data : []; - if (requestId === catalogRequestId) catalog = nextCatalog; - return nextCatalog; - }) - .finally(() => { - if (catalogPromise === request) catalogPromise = null; - }); + request = (async () => { + try { + return await requestModCatalog(requestId, forceRefresh); + } catch (error) { + // A slow connection makes the native side give up on the first screen + // while it keeps downloading: wait for that download and retry once, so + // the catalog is not lost to an 8s timeout. A real failure marks the + // catalog offline, and retrying that would only fail again. + const superseded = requestId !== catalogRequestId; + const offline = await callRemote("is_catalog_offline").catch( + () => false, + ); + if ( + superseded || + offline || + !(await waitForCatalogDownload(isCatalogDownloading, sleep)) + ) { + throw error; + } + return await requestModCatalog(requestId, false); + } + })().finally(() => { + if (catalogPromise === request) catalogPromise = null; + }); catalogPromise = request; return request; }; diff --git a/src/celemod-ui/src/catalogDownload.test.ts b/src/celemod-ui/src/catalogDownload.test.ts new file mode 100644 index 0000000..a0cfb70 --- /dev/null +++ b/src/celemod-ui/src/catalogDownload.test.ts @@ -0,0 +1,43 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { waitForCatalogDownload } from "./catalogDownload"; + +const never = () => Promise.resolve(false); +const always = () => Promise.resolve(true); +const noop = () => Promise.resolve(); + +test("returns immediately when no catalog download is running", async () => { + let waits = 0; + const settled = await waitForCatalogDownload(never, () => { + waits += 1; + return Promise.resolve(); + }); + assert.equal(settled, true); + assert.equal(waits, 0); +}); + +test("waits for a slow download to settle", async () => { + let polls = 0; + let waits = 0; + const downloading = () => { + polls += 1; + return Promise.resolve(polls < 3); + }; + const settled = await waitForCatalogDownload(downloading, () => { + waits += 1; + return Promise.resolve(); + }); + assert.equal(settled, true); + assert.equal(polls, 3); + assert.equal(waits, 2); +}); + +test("gives up after the polling budget instead of hanging", async () => { + let waits = 0; + const settled = await waitForCatalogDownload(always, () => { + waits += 1; + return Promise.resolve(); + }, 4, 25); + assert.equal(settled, false); + assert.equal(waits, 4); +}); diff --git a/src/celemod-ui/src/catalogDownload.ts b/src/celemod-ui/src/catalogDownload.ts new file mode 100644 index 0000000..e7b3a78 --- /dev/null +++ b/src/celemod-ui/src/catalogDownload.ts @@ -0,0 +1,26 @@ +/** + * The native side stops blocking the first screen on the Mod catalog after a + * few seconds, but it keeps a slow download running and publishes it to the + * on-disk cache once it lands. These helpers let callers wait for that download + * to settle instead of failing with an empty catalogue. + */ +export const CATALOG_DOWNLOAD_POLL_INTERVAL_MS = 3000; +/** Matches the native catalog download deadline (60s) when polling. */ +export const CATALOG_DOWNLOAD_POLL_LIMIT = 20; + +/** + * Waits until no catalog download is running anymore (or the polling budget is + * used up). Returns `true` when the download settled, so the caller can retry. + */ +export const waitForCatalogDownload = async ( + downloading: () => Promise, + wait: (ms: number) => Promise, + limit: number = CATALOG_DOWNLOAD_POLL_LIMIT, + intervalMs: number = CATALOG_DOWNLOAD_POLL_INTERVAL_MS, +): Promise => { + for (let attempt = 0; attempt < limit; attempt += 1) { + if (!(await downloading())) return true; + await wait(intervalMs); + } + return false; +}; diff --git a/src/celemod-ui/src/context/modManage.tsx b/src/celemod-ui/src/context/modManage.tsx index 86b5981..ba7151c 100644 --- a/src/celemod-ui/src/context/modManage.tsx +++ b/src/celemod-ui/src/context/modManage.tsx @@ -1,5 +1,5 @@ import _i18n from "src/i18n"; -import { callRemote } from "../utils"; +import { callRemote, sleep } from "../utils"; import { useGamePath, initGamePath, @@ -10,6 +10,8 @@ import { useEffect, useContext, useState } from "react"; import { createPopup, PopupContext } from "src/components/Popup"; import { ProgressIndicator } from "src/components/Progress"; import { syncModsWatcher } from "../modsWatcher"; +import { waitForCatalogDownload } from "../catalogDownload"; +import { isCatalogDownloading } from "../api/modCatalog"; export const createModManageContext = () => { initModComments(); @@ -141,6 +143,18 @@ export const createModManageContext = () => { await ctx.reloadMods(); const isOffline = await callRemote("is_catalog_offline"); if (!cancelled) setOffline(isOffline); + // A slow connection can still be downloading the catalog after the + // first screen stopped waiting for it: refresh the Mod list once it + // lands instead of leaving the Mod metadata missing. + if (!isOffline && (await isCatalogDownloading())) { + void waitForCatalogDownload(isCatalogDownloading, sleep).then( + (settled) => { + if (settled && !cancelled) { + void ctx.reloadMods().catch(console.error); + } + }, + ); + } })() .then(() => { popup?.hide(); diff --git a/src/celemod-ui/src/tauri/commands.ts b/src/celemod-ui/src/tauri/commands.ts index 7132592..2692f29 100644 --- a/src/celemod-ui/src/tauri/commands.ts +++ b/src/celemod-ui/src/tauri/commands.ts @@ -125,6 +125,7 @@ const parameterNames: Record = { write_frontend_log: ["level", "message"], set_catalog_offline: ["offline"], is_catalog_offline: [], + is_mod_catalog_downloading: [], is_using_cache: [], configure_mod_cache: ["ttlSeconds"], get_mod_catalog: ["forceRefresh"],