diff --git a/apps/readest-app/src-tauri/build.rs b/apps/readest-app/src-tauri/build.rs index 39878be1..b1059121 100644 --- a/apps/readest-app/src-tauri/build.rs +++ b/apps/readest-app/src-tauri/build.rs @@ -12,10 +12,52 @@ fn main() { } propagate_sentry_dsn(); + propagate_app_version(); tauri_build::build() } +/// Bake the app version from `package.json` into the crate as `READEST_APP_VERSION` +/// (read back via `option_env!`). Sentry keys its release/environment off this +/// rather than `CARGO_PKG_VERSION`, because the crate version in `Cargo.toml` is +/// not kept in sync with the app version (and only `package.json` carries the +/// nightly `-YYYYMMDDHH` stamp). Absent/unparseable => unset, so the Rust code +/// falls back to the crate version. +fn propagate_app_version() { + let package_json = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()) + .join("..") + .join("package.json"); + println!("cargo:rerun-if-changed={}", package_json.display()); + + if let Some(version) = read_json_string_field(&package_json, "version") { + println!("cargo:rustc-env=READEST_APP_VERSION={version}"); + } +} + +/// Read a top-level `"key": "value"` string from a JSON file without pulling in a +/// JSON parser. Returns the first match; `None` if the file/key is absent or the +/// value is empty. `package.json`'s own `"version"` is the first `"version"` key. +fn read_json_string_field(path: &Path, key: &str) -> Option { + let contents = fs::read_to_string(path).ok()?; + let needle = format!("\"{key}\""); + for line in contents.lines() { + let Some(rest) = line.trim_start().strip_prefix(&needle) else { + continue; + }; + let value = rest + .trim_start() + .strip_prefix(':')? + .trim() + .trim_end_matches(',') + .trim() + .trim_matches('"'); + if !value.is_empty() { + return Some(value.to_string()); + } + } + None +} + /// Bake the Sentry DSN into the crate at build time via `cargo:rustc-env`, so /// `option_env!("SENTRY_DSN")` (and, on iOS, the `readest_sentry_dsn` FFI) sees /// it. Precedence: an existing `SENTRY_DSN` in the environment (CI secret / shell diff --git a/apps/readest-app/src-tauri/src/lib.rs b/apps/readest-app/src-tauri/src/lib.rs index 996afcf9..bfbf3c27 100644 --- a/apps/readest-app/src-tauri/src/lib.rs +++ b/apps/readest-app/src-tauri/src/lib.rs @@ -313,10 +313,31 @@ pub fn run() { sentry::init(( dsn, sentry::ClientOptions { - release: sentry::release_name!(), + release: Some(sentry_config::sentry_release().into()), environment: Some(sentry_config::sentry_environment().into()), traces_sample_rate: 0.0, send_default_pii: false, + // On Android the context integration reads `uname()` and reports + // the OS as "Linux"; relabel it "Android" (and recover the Android + // version from the kernel string) so events group correctly. + before_send: Some(std::sync::Arc::new(|mut event| { + if let Some(sentry::protocol::Context::Os(os)) = event.contexts.get_mut("os") { + if let Some(name) = sentry_config::corrected_os_name( + std::env::consts::OS, + os.name.as_deref(), + ) { + os.name = Some(name.to_owned()); + if let Some(version) = os + .version + .as_deref() + .and_then(sentry_config::android_version_from_uname) + { + os.version = Some(version); + } + } + } + Some(event) + })), ..Default::default() }, )) diff --git a/apps/readest-app/src-tauri/src/sentry_config.rs b/apps/readest-app/src-tauri/src/sentry_config.rs index ad69b569..cd794eb8 100644 --- a/apps/readest-app/src-tauri/src/sentry_config.rs +++ b/apps/readest-app/src-tauri/src/sentry_config.rs @@ -27,9 +27,53 @@ pub fn environment_for_version(version: &str) -> &'static str { "production" } +/// The user-facing application version, taken from `package.json` at build time +/// (baked as `READEST_APP_VERSION` by `build.rs`). Falls back to the crate version +/// only if the bake is missing. The crate version (`Cargo.toml`, e.g. `0.2.2`) is +/// not kept in lockstep with the app, so Sentry must key its release and +/// environment off the app version instead of `CARGO_PKG_VERSION`. +pub fn app_version() -> &'static str { + option_env!("READEST_APP_VERSION").unwrap_or(env!("CARGO_PKG_VERSION")) +} + +/// Joins a crate name and version into Sentry's `name@version` release format +/// (e.g. `Readest@0.11.17`), matching `sentry::release_name!()`'s shape. +pub fn release_name(name: &str, version: &str) -> String { + format!("{name}@{version}") +} + +/// Sentry `release` for the current build, keyed off the app version. +pub fn sentry_release() -> String { + release_name(env!("CARGO_PKG_NAME"), app_version()) +} + /// Sentry `environment` for the current build. pub fn sentry_environment() -> &'static str { - environment_for_version(env!("CARGO_PKG_VERSION")) + environment_for_version(app_version()) +} + +/// The Rust SDK's context integration derives the OS name from `uname()`, which +/// reports "Linux" on Android (Android runs a Linux kernel). Given the build's +/// `target_os` and the already-detected name, return the name Sentry should show, +/// or `None` to leave it unchanged. Only Android is remapped; the native +/// sentry-android/-cocoa SDKs report other platforms correctly on their own. +pub fn corrected_os_name(target_os: &str, detected: Option<&str>) -> Option<&'static str> { + (target_os == "android" && detected != Some("Android")).then_some("Android") +} + +/// Extracts the Android platform version from `uname`'s kernel release string +/// (e.g. `6.1.162-android14-11-g5e8b0cffebd1-ab15202165` -> `"14"`). `uname` +/// reports the Linux kernel version, not the Android version, but Android kernels +/// embed an `android` token. Returns `None` when no such token is present. +pub fn android_version_from_uname(release: &str) -> Option { + let digits = release + .split(['-', '_']) + .find_map(|token| token.strip_prefix("android"))?; + if !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit()) { + Some(digits.to_string()) + } else { + None + } } /// C-ABI accessor for the compile-time Sentry DSN, used by the iOS native @@ -51,7 +95,10 @@ pub extern "C" fn readest_sentry_dsn() -> *const std::os::raw::c_char { #[cfg(test)] mod tests { - use super::{dsn_from_env, environment_for_version}; + use super::{ + android_version_from_uname, corrected_os_name, dsn_from_env, environment_for_version, + release_name, + }; #[test] fn dsn_is_none_when_unset_or_blank() { @@ -79,4 +126,43 @@ mod tests { assert_eq!(environment_for_version("0.11.17-rc.1"), "production"); assert_eq!(environment_for_version("1.0.0-2026"), "production"); } + + #[test] + fn release_name_joins_crate_name_and_app_version() { + assert_eq!(release_name("Readest", "0.11.17"), "Readest@0.11.17"); + } + + #[test] + fn os_name_is_corrected_to_android_on_android_target() { + assert_eq!(corrected_os_name("android", Some("Linux")), Some("Android")); + // No detected name still yields Android on an Android build. + assert_eq!(corrected_os_name("android", None), Some("Android")); + } + + #[test] + fn os_name_is_left_unchanged_off_android_or_already_correct() { + assert_eq!(corrected_os_name("android", Some("Android")), None); + assert_eq!(corrected_os_name("linux", Some("Linux")), None); + assert_eq!(corrected_os_name("macos", Some("macOS")), None); + assert_eq!(corrected_os_name("windows", Some("Windows")), None); + } + + #[test] + fn android_version_parsed_from_kernel_release() { + assert_eq!( + android_version_from_uname("6.1.162-android14-11-g5e8b0cffebd1-ab15202165"), + Some("14".to_string()) + ); + assert_eq!( + android_version_from_uname("5.10.101-android12-9-00001"), + Some("12".to_string()) + ); + } + + #[test] + fn android_version_is_none_without_android_token() { + assert_eq!(android_version_from_uname("6.1.162"), None); + assert_eq!(android_version_from_uname(""), None); + assert_eq!(android_version_from_uname("6.1.162-androidX"), None); + } } diff --git a/apps/readest-app/src/__tests__/libs/storage.test.ts b/apps/readest-app/src/__tests__/libs/storage.test.ts new file mode 100644 index 00000000..24f3a610 --- /dev/null +++ b/apps/readest-app/src/__tests__/libs/storage.test.ts @@ -0,0 +1,45 @@ +import { describe, test, expect, vi, beforeEach } from 'vitest'; + +vi.mock('@/services/environment', () => ({ + getAPIBaseUrl: () => 'https://api.test', + isWebAppPlatform: () => false, +})); +vi.mock('@/utils/access', () => ({ getUserID: vi.fn() })); +vi.mock('@/utils/fetch', () => ({ fetchWithAuth: vi.fn() })); +vi.mock('@/utils/transfer', () => ({ + tauriUpload: vi.fn(), + tauriDownload: vi.fn(), + webUpload: vi.fn(), + webDownload: vi.fn(), +})); + +import { deleteFile } from '@/libs/storage'; +import { getUserID } from '@/utils/access'; +import { fetchWithAuth } from '@/utils/fetch'; + +describe('deleteFile (cloud) — best-effort cleanup (READEST-5)', () => { + beforeEach(() => vi.clearAllMocks()); + + test('resolves without throwing when the delete request fails', async () => { + vi.mocked(getUserID).mockResolvedValue('user-1'); + vi.mocked(fetchWithAuth).mockRejectedValue(new Error('network down')); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + // Callers dispatch this without awaiting; a throw here becomes an unhandled + // rejection. It must swallow the failure and just log it. + await expect(deleteFile('books/x.epub')).resolves.toBeUndefined(); + expect(warn).toHaveBeenCalled(); + + warn.mockRestore(); + }); + + test('resolves without throwing when the user is not authenticated', async () => { + vi.mocked(getUserID).mockResolvedValue(null); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await expect(deleteFile('books/x.epub')).resolves.toBeUndefined(); + expect(fetchWithAuth).not.toHaveBeenCalled(); + + warn.mockRestore(); + }); +}); diff --git a/apps/readest-app/src/__tests__/store/transfer-store.test.ts b/apps/readest-app/src/__tests__/store/transfer-store.test.ts index 77c225cd..118fb37e 100644 --- a/apps/readest-app/src/__tests__/store/transfer-store.test.ts +++ b/apps/readest-app/src/__tests__/store/transfer-store.test.ts @@ -76,6 +76,16 @@ describe('transferStore', () => { useTransferStore.getState().updateTransferProgress('nope', 10, 10, 100, 5); expect(useTransferStore.getState().transfers).toEqual(before); }); + + test('re-applying identical values keeps the same state reference (READEST-2)', () => { + const id = useTransferStore.getState().addTransfer('h', 'B', 'upload'); + useTransferStore.getState().updateTransferProgress(id, 50, 500, 1000, 100); + const before = useTransferStore.getState().transfers; + // A repeated write with unchanged values must not allocate a new state, + // otherwise subscribers re-render and can drive an update loop. + useTransferStore.getState().updateTransferProgress(id, 50, 500, 1000, 100); + expect(useTransferStore.getState().transfers).toBe(before); + }); }); // ── setTransferStatus ──────────────────────────────────────────── diff --git a/apps/readest-app/src/app/reader/components/ReadingStatsTracker.tsx b/apps/readest-app/src/app/reader/components/ReadingStatsTracker.tsx index 01101031..84b2c4e7 100644 --- a/apps/readest-app/src/app/reader/components/ReadingStatsTracker.tsx +++ b/apps/readest-app/src/app/reader/components/ReadingStatsTracker.tsx @@ -14,6 +14,13 @@ import { isSyncCategoryEnabled } from '@/services/sync/syncCategories'; const nowSec = () => Math.floor(Date.now() / 1000); +// Statistics are best-effort telemetry: a failed write or sync (e.g. the +// statistics DB torn down mid-flight on app teardown -> "database ... not +// loaded", Sentry READEST-6) must never surface as an unhandled rejection. +const runBestEffort = (work: Promise): void => { + void work.catch((err) => console.warn('[stats] background operation failed:', err)); +}; + export default function ReadingStatsTracker({ bookKey }: { bookKey: string }) { const { appService } = useEnv(); // Progress lives in readerProgressStore, not readerStore.viewStates. @@ -41,7 +48,7 @@ export default function ReadingStatsTracker({ bookKey }: { bookKey: string }) { if (pushTimerRef.current) clearTimeout(pushTimerRef.current); pushTimerRef.current = setTimeout(() => { const db = dbRef.current; - if (db) void pushStats(db, new SyncClient()); + if (db) runBestEffort(pushStats(db, new SyncClient())); }, 10_000); }; @@ -51,7 +58,7 @@ export default function ReadingStatsTracker({ bookKey }: { bookKey: string }) { StatisticsDb.open(appService).then((db) => { if (cancelled) return; dbRef.current = db; - if (syncEnabled()) void pullStats(db, new SyncClient()); + if (syncEnabled()) runBestEffort(pullStats(db, new SyncClient())); }); return () => { cancelled = true; @@ -60,15 +67,20 @@ export default function ReadingStatsTracker({ bookKey }: { bookKey: string }) { }, [appService]); // Persist flushed events into the statistics DB. - const persist = (events: FlushedEvent[]): Promise => { + const persist = async (events: FlushedEvent[]): Promise => { const db = dbRef.current; - if (!db || !bookMd5 || events.length === 0) return Promise.resolve(); - return (async () => { + if (!db || !bookMd5 || events.length === 0) return; + try { const idBook = await db.upsertBook({ bookMd5, title, authors }); for (const e of events) await db.insertPageEvent(idBook, e); await db.recomputeBookTotals(idBook); schedulePush(); - })(); + } catch (err) { + // The statistics DB can be closed mid-write on app/tab teardown + // ("database ... not loaded", Sentry READEST-6). Best-effort: log and + // never reject, so the fire-and-forget dispatch sites stay safe. + console.warn('[stats] failed to persist reading events:', err); + } }; const armIdle = () => { @@ -108,11 +120,12 @@ export default function ReadingStatsTracker({ bookKey }: { bookKey: string }) { return () => { if (idleRef.current) clearTimeout(idleRef.current); if (pushTimerRef.current) clearTimeout(pushTimerRef.current); - const closePersist = persist(coreRef.current.onClose(nowSec())); - void closePersist.then(() => { - if (syncEnabled() && dbRef.current) return pushStats(dbRef.current, new SyncClient()); - return undefined; - }); + runBestEffort( + persist(coreRef.current.onClose(nowSec())).then(() => { + if (syncEnabled() && dbRef.current) return pushStats(dbRef.current, new SyncClient()); + return undefined; + }), + ); }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [bookMd5]); diff --git a/apps/readest-app/src/libs/storage.ts b/apps/readest-app/src/libs/storage.ts index de84b8c4..3482f3d5 100644 --- a/apps/readest-app/src/libs/storage.ts +++ b/apps/readest-app/src/libs/storage.ts @@ -230,8 +230,10 @@ export const deleteFile = async (filePath: string) => { method: 'DELETE', }); } catch (error) { - console.error('File deletion failed:', error); - throw new Error('File deletion failed'); + // Best-effort cloud cleanup: removing the remote copy is non-critical and + // callers dispatch this without awaiting, so throwing here surfaces as an + // unhandled promise rejection (Sentry READEST-5). Log and swallow instead. + console.warn('File deletion failed:', error); } }; diff --git a/apps/readest-app/src/store/transferStore.ts b/apps/readest-app/src/store/transferStore.ts index c896916c..3496d2be 100644 --- a/apps/readest-app/src/store/transferStore.ts +++ b/apps/readest-app/src/store/transferStore.ts @@ -198,6 +198,18 @@ export const useTransferStore = create((set, get) => ({ const transfer = state.transfers[transferId]; if (!transfer) return state; + // No-op when nothing changed: re-applying identical progress would + // otherwise allocate a new state on every call and re-render every + // subscriber, which can sustain a render/update loop (Sentry READEST-2). + if ( + transfer.progress === progress && + transfer.transferredBytes === transferred && + transfer.totalBytes === total && + transfer.transferSpeed === speed + ) { + return state; + } + return { transfers: { ...state.transfers,