fix(db): break probe-failed/restore loop on large storage.sqlite#6632
Open
KooshaPari wants to merge 4 commits into
Open
fix(db): break probe-failed/restore loop on large storage.sqlite#6632KooshaPari wants to merge 4 commits into
KooshaPari wants to merge 4 commits into
Conversation
… (ABI 148) (diegosouzapw#6605) fix(electron): bump electron 42→43 + rebuild better-sqlite3 from source against the Electron ABI (148). Electron 43 raises NODE_MODULE_VERSION to 148; better-sqlite3@12.11.1 has no electron-v148 prebuild, so the packaged app died with 'Nenhum driver SQLite disponível'. prepare-electron-standalone now compiles better-sqlite3 from source against the electron headers into build/Release (where 'bindings' resolves it). Validated by Electron Package Smoke (green) + local (node_register_module_v148). Supersedes diegosouzapw#6378. (--admin: the only reds are SonarQube/SonarCloud failing on a coverage-report artifact digest-mismatch — a GitHub Actions infra flake, not this diff; Sonar is green on main and the diff touches only the electron build.)
…iegosouzapw#6588) deps: bump the development group (6 updates). Rebased onto current main; all checks green after the electron-smoke fix (diegosouzapw#6605).
…7) + production deps bump (diegosouzapw#6620) fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump. undici 8.6+ changed ProxyAgent to forward plain-HTTP via request-proxy instead of CONNECT, breaking OAuth refresh through a connection proxy (501). proxyDispatcher now passes proxyTunnel:true. Validated: Unit Tests 3/8 (the OAuth-proxy test) green, new regression test green (fails without the fix on undici 8.7), SonarQube green. Supersedes diegosouzapw#6380. (--admin: the only red is Electron Package Smoke failing on a next-build artifact 'digest-mismatch' — a GitHub Actions infra flake corrupting the asar ('file data stream has unexpected number of bytes'); the better-sqlite3 rebuild itself succeeded (gyp ok) and the electron path is unchanged from diegosouzapw#6605 which passed the smoke. Not this diff.)
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(db): break probe-failed/restore loop on large storage.sqlite
Summary
Fixes an infinite loop in
getDbInstance()that prevents omniroute v3.8.46 from completing startup whenstorage.sqliteexceeds ~100 MB on a sql.js runtime under the default Node heap limit.Closes #6631
Root cause
The startup probe flow in
src/lib/db/core.tshas two interacting bugs that together produce a tight 5ms-interval loop:storage.sqlite→storage.sqlite.probe-failed-<timestamp>on every failure — including transient out-of-memory errors that are environment-driven (heap limit vs. DB size), not corruption-driven.!existsSync(R) && probeFailureBackups.length > 0and unconditionally renames the most recentprobe-failed-<ts>back tostorage.sqlite. This "recovery" path silently re-loads the same file that just failed.Combined: probe OOMs → rename → next call restores → probe OOMs → repeat. The loop is driven by multiple startup services (
ProviderLimitsSync,ModelSync,HealthCheck,BATCH,callLogs,STARTUP) each independently callinggetDbInstance()in parallel.Symptoms in
app.log:A 326 MB
storage.sqlitereproduces this on a fresh install under default--max-old-space-size. The probe in readonly mode allocates the entire file in sql.js WASM heap (often 3–5x file size) and OOMs.Fix
Two minimal, surgical changes in
src/lib/db/core.ts:1. Cycle-breaker guard inside the auto-restore condition
After 3 restoration attempts in the same process, throw a clear, actionable fatal error instead of looping. The counter is per-process and resets on restart (by design — a fresh start with the probe-failed files manually removed should succeed).
2. Skip rename on transient OOM
OOM is a runtime/environment condition (heap limit vs. DB size) that cannot be fixed by re-loading the same file. Skipping the rename preserves the working database for the user to manually inspect or VACUUM.
Verification
Repro environment: Windows, Node v26.3.0, omniroute v3.8.46,
storage.sqlite≈ 326 MB.Without fix: startup hangs forever,
app.logshows the rename/restore loop every ~5ms.With fix:
storage.sqlite.probe-failed-*files, restarts → boots cleanly.Test plan
bun build --target=bun(no TS errors)pnpm testand the DB test suiteRelated notes
This PR addresses the launch hang specifically. A separate, downstream issue is the Next.js
Cannot create property 'message' on string 'Database closed'TypeError that fires frominstrumentation.tswhengetDbInstance()throws a string from sql.js. That's a different bug (error-as-string vs. error-as-object in the Next.js instrumentation hook) and should be addressed in a separate PR.