akai-tools/readest
2
0
Fork 0
forked from akai/readest
Code Pull Requests Activity

fix: more production crashes (View Transition noise, book-dir race, stats transaction) (#4962)

Browse Source 
* fix(sentry): drop more benign View Transition rejections

Broaden is_ignored_browser_error to also drop "Transition was skipped"
(navigation superseded, READEST-F) and "aborted because of invalid state"
(READEST-G), matched case-insensitively alongside the existing hidden-tab case.
These are expected browser behavior — the navigation completes, only the
animation is skipped/aborted. A transition timeout stays visible (real perf
signal, handled separately).

Fixes READEST-F
Fixes READEST-G

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(library): create the book directory idempotently

Importing a book did a check-then-create with a non-recursive createDir. Two
concurrent imports of the same book both pass the exists check, then the second
create fails — on Windows with "Cannot create a file when that file already
exists" (Sentry READEST-H). Use a recursive create (create_dir_all), a no-op
when the directory already exists.

Fixes READEST-H

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(stats): serialize applyRemoteEvents to avoid nested transactions

The statistics connection is shared across ReadingStatsTracker instances (split
view). applyRemoteEvents runs a manual BEGIN/COMMIT that the per-op native
connection lock does not make atomic, so two concurrent pulls opened a BEGIN
inside a BEGIN ("cannot start a transaction within a transaction", Sentry
READEST-N). Serialize applyRemoteEvents against itself with a small promise
mutex. Adds a regression test that fails without the guard.

Fixes READEST-N

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-07-07 00:05:35 +09:00
committed by GitHub
parent 2a837cb50d
commit a02b236e97
4 changed files with 79 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/readest-app/src-tauri/src/sentry_config.rs
+21 -7
View File
@@ -76,13 +76,18 @@ pub fn android_version_from_uname(release: &str) -> Option<String> {
}
}
/// A known-benign browser error that is expected behavior, not an app bug, and
/// only adds noise to crash reporting: the View Transition API skips a
/// transition when the tab is hidden. Matched on the exception value so it is
/// dropped in `before_send`. NOTE: a transition *timeout* abort is deliberately
/// NOT ignored — a slow DOM update can signal a real performance problem.
/// Known-benign browser errors that are expected behavior, not app bugs, and
/// only add noise to crash reporting: the View Transition API skips a transition
/// when the tab is hidden or the navigation is superseded, and aborts it when the
/// document is in an invalid state. These arrive as unhandled rejections while
/// the navigation itself still completes. Matched (case-insensitively) on the
/// exception value so they are dropped in `before_send`. NOTE: a transition
/// *timeout* abort is deliberately NOT ignored — a slow DOM update can signal a
/// real performance problem.
pub fn is_ignored_browser_error(value: &str) -> bool {
value.contains("View transition was skipped because document visibility state is hidden")
let value = value.to_lowercase();
value.contains("transition was skipped")
|| value.contains("transition was aborted because of invalid state")
}
/// The WebView (engine, major-version), set once at startup when the app reports
@@ -216,10 +221,19 @@ mod tests {
}
#[test]
fn ignores_benign_hidden_view_transition_error() {
fn ignores_benign_view_transition_errors() {
// Skipped because the tab is hidden (READEST-7).
assert!(is_ignored_browser_error(
"InvalidStateError: View transition was skipped because document visibility state is hidden."
));
// Skipped because the navigation was superseded (READEST-F).
assert!(is_ignored_browser_error(
"AbortError: Transition was skipped"
));
// Aborted because the document was in an invalid state (READEST-G).
assert!(is_ignored_browser_error(
"InvalidStateError: Transition was aborted because of invalid state"
));
}
#[test]
apps/readest-app/src/__tests__/statistics/statisticsDb.test.ts
+16
View File
@@ -103,6 +103,22 @@ describe('StatisticsDb', () => {
expect(book!.total_read_time).toBe(8);
});
it('serializes concurrent applyRemoteEvents without nesting transactions (READEST-N)', async () => {
// Two pulls racing on the shared connection (split-view trackers) must not
// open a BEGIN inside a BEGIN ("cannot start a transaction within a transaction").
const a = stats.applyRemoteEvents(
[{ bookMd5: 'ra', title: 'RA', authors: '' }],
[{ bookMd5: 'ra', page: 1, startTime: 400, duration: 3, totalPages: 10 }],
);
const b = stats.applyRemoteEvents(
[{ bookMd5: 'rb', title: 'RB', authors: '' }],
[{ bookMd5: 'rb', page: 1, startTime: 401, duration: 4, totalPages: 10 }],
);
await expect(Promise.all([a, b])).resolves.toBeDefined();
expect((await stats.getBookByMd5('ra'))!.total_read_time).toBe(3);
expect((await stats.getBookByMd5('rb'))!.total_read_time).toBe(4);
});
it('reads and writes sync cursors', async () => {
expect(await stats.getCursor('push')).toBe(0);
await stats.setCursor('push', 1234);
apps/readest-app/src/services/bookService.ts
+6 -3
View File
@@ -537,9 +537,12 @@ export async function importBook(
existingBook.downloadedAt = Date.now();
}
if (!(await fs.exists(getDir(book), 'Books'))) {
await fs.createDir(getDir(book), 'Books');
}
// Idempotent create (recursive): a plain createDir defaults to
// non-recursive and throws if the dir already exists, and the check-then-
// create above races two concurrent imports of the same book — on Windows
// the loser fails with "Cannot create a file when that file already exists"
// (Sentry READEST-H). create_dir_all is a no-op when the dir exists.
await fs.createDir(getDir(book), 'Books', true);
const bookFilename = getLocalBookFilename(book);
const willWriteBookFile =
saveBook &&
apps/readest-app/src/services/statistics/statisticsDb.ts
+36 -20
View File
@@ -43,6 +43,9 @@ function bindLifecycle(): void {
* leaves this class.
*/
export class StatisticsDb {
// Serializes applyRemoteEvents so two concurrent pulls can't nest BEGINs.
private applyRemoteLock: Promise<void> = Promise.resolve();
private constructor(private readonly db: DatabaseService) {}
/** Production entry point — opens + migrates statistics.db (per-tab singleton). */
@@ -187,29 +190,42 @@ export class StatisticsDb {
async applyRemoteEvents(books: StatBook[], events: PageStatEvent[]): Promise<void> {
if (books.length === 0 && events.length === 0) return;
// One transaction for the whole pulled batch: a single commit instead of
// O(rows) fsyncs, and the apply is atomic (a failed pull leaves no partial
// state). Critical when a fresh device backfills tens of thousands of rows.
await this.db.execute('BEGIN');
// Serialize against other pulls: the statistics connection is shared across
// ReadingStatsTracker instances (split view), and a second concurrent pull
// would open a BEGIN inside this one's still-open BEGIN ("cannot start a
// transaction within a transaction", Sentry READEST-N). The per-op native
// lock can't make this multi-statement transaction atomic on its own.
const prev = this.applyRemoteLock;
let release!: () => void;
this.applyRemoteLock = new Promise<void>((resolve) => (release = resolve));
await prev;
try {
const idByMd5 = new Map<string, number>();
for (const b of books) idByMd5.set(b.bookMd5, await this.upsertBook(b));
// Books referenced only by events (no metadata record) get a placeholder row.
const touched = new Set<number>();
for (const e of events) {
let id = idByMd5.get(e.bookMd5);
if (id === undefined) {
id = await this.ensureBookId(e.bookMd5);
idByMd5.set(e.bookMd5, id);
// One transaction for the whole pulled batch: a single commit instead of
// O(rows) fsyncs, and the apply is atomic (a failed pull leaves no partial
// state). Critical when a fresh device backfills tens of thousands of rows.
await this.db.execute('BEGIN');
try {
const idByMd5 = new Map<string, number>();
for (const b of books) idByMd5.set(b.bookMd5, await this.upsertBook(b));
// Books referenced only by events (no metadata record) get a placeholder row.
const touched = new Set<number>();
for (const e of events) {
let id = idByMd5.get(e.bookMd5);
if (id === undefined) {
id = await this.ensureBookId(e.bookMd5);
idByMd5.set(e.bookMd5, id);
}
await this.insertPageEvent(id, e);
touched.add(id);
}
await this.insertPageEvent(id, e);
touched.add(id);
for (const id of touched) await this.recomputeBookTotals(id);
await this.db.execute('COMMIT');
} catch (err) {
await this.db.execute('ROLLBACK');
throw err;
}
for (const id of touched) await this.recomputeBookTotals(id);
await this.db.execute('COMMIT');
} catch (err) {
await this.db.execute('ROLLBACK');
throw err;
} finally {
release();
}
}