fix: Sentry production hardening (release/OS tags, unhandled-rejection & render-loop guards) (#4929)

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<String> {
+    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

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()
             },
         ))

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<N>` token. Returns `None` when no such token is present.
+pub fn android_version_from_uname(release: &str) -> Option<String> {
+    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);
+    }
 }

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();
+  });
+});

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 ────────────────────────────────────────────

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<unknown>): 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<void> => {
+  const persist = async (events: FlushedEvent[]): Promise<void> => {
     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]);

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);
   }
 };
 

apps/readest-app/src/store/transferStore.ts

@@ -198,6 +198,18 @@ export const useTransferStore = create<TransferState>((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,