docs: add v3.3.3+ hardening invariants and ordering guards

- Added comprehensive INVARIANTS documentation to _applyPragmasOnExistingConnection()
- Added ORDERING GUARD documentation to _initialize() and _ensureConnection()
- Enhanced iOS compatibility logging for WAL mode failures
- Added invariant reference comments throughout PRAGMA code paths
- Improved error handling with non-fatal WAL failure handling
- All invariants documented to prevent regression:
  1. foreign_keys via run()
  2. journal_mode via query() with iOS compatibility
  3. busy_timeout via query() with fallback
  4. synchronous skipped in fallback path
  5. this.db must be set before returning

This locks in the v3.3.3 fix and ensures stability on Android and iOS.
This commit is contained in:
Matthew Raymer
2025-11-07 12:28:41 +00:00
parent c26b8daaf7
commit 471bdd6b92

View File

@@ -146,10 +146,15 @@ export class CapacitorPlatformService
} }
/** /**
* Internal initialization method (v3.3) * Internal initialization method (v3.3.3)
* *
* Order: PRAGMAs (temp connection, no transactions) → connect → open → migrations (single txn) → verify * Order: PRAGMAs (temp connection, no transactions) → connect → open → migrations (single txn) → verify
* *
* ORDERING GUARD (v3.3.3+):
* - PRAGMAs MUST run before any execute/executeSet operations
* - This ensures database-level settings are applied before migrations
* - If ordering changes, add unit test that fails on execute before PRAGMAs
*
* @private Internal method - use initializeDatabase() instead * @private Internal method - use initializeDatabase() instead
*/ */
private async _initialize(): Promise<void> { private async _initialize(): Promise<void> {
@@ -163,6 +168,7 @@ export class CapacitorPlatformService
try { try {
// Phase 1: Apply PRAGMAs on temporary connection (MUST be before main connection) // Phase 1: Apply PRAGMAs on temporary connection (MUST be before main connection)
// This ensures PRAGMAs run outside any transaction context // This ensures PRAGMAs run outside any transaction context
// ORDERING GUARD: No execute/executeSet operations should occur before this phase
logger.debug( logger.debug(
"[CapacitorPlatformService] Starting PRAGMA phase (before main connection)", "[CapacitorPlatformService] Starting PRAGMA phase (before main connection)",
); );
@@ -281,12 +287,17 @@ export class CapacitorPlatformService
* *
* Implements retrieve-first, create-second, close+create-last pattern. * Implements retrieve-first, create-second, close+create-last pattern.
* Short-circuits if fallback PRAGMA method already set this.db. * Short-circuits if fallback PRAGMA method already set this.db.
*
* ORDERING GUARD (v3.3.3+):
* - This method MUST be called AFTER _applyPragmasOnTempConnection()
* - PRAGMAs must run before any execute/executeSet operations
* - If this.db is already set by fallback PRAGMA path, return immediately
*/ */
private async _ensureConnection(): Promise<void> { private async _ensureConnection(): Promise<void> {
// 1) If fallback already set a live handle, use it // 1) Short-circuit: If fallback PRAGMA method already set this.db, use it (invariant #3)
if (this.db) { if (this.db) {
logger.log( logger.log(
"[CapacitorPlatformService] [DB] ensure: already have connection", "[CapacitorPlatformService] [DB] ensure: already have connection (from PRAGMA fallback)",
); );
return; return;
} }
@@ -469,7 +480,7 @@ export class CapacitorPlatformService
// Foreign keys (safe via run) // Foreign keys (safe via run)
await pragmaConn.run("PRAGMA foreign_keys = ON;", []); await pragmaConn.run("PRAGMA foreign_keys = ON;", []);
// journal_mode via query (returns a row) // journal_mode via query (returns a row) - invariant #2
let journalMode = "unknown"; let journalMode = "unknown";
if (platform !== "web") { if (platform !== "web") {
try { try {
@@ -477,16 +488,33 @@ export class CapacitorPlatformService
const row = res.values?.[0] as Record<string, unknown> | undefined; const row = res.values?.[0] as Record<string, unknown> | undefined;
const key = row ? Object.keys(row)[0] : undefined; const key = row ? Object.keys(row)[0] : undefined;
journalMode = (key ? (row![key] as string) : undefined) ?? "unknown"; journalMode = (key ? (row![key] as string) : undefined) ?? "unknown";
} catch {
// leave as unknown // iOS compatibility: WAL may be refused on read-only bundles
// Log but don't throw - device may not support WAL
if (journalMode !== "wal") {
logger.debug(
`[CapacitorPlatformService] WAL mode not effective (got: ${journalMode}), continuing with default`,
);
}
} catch (walError) {
// Non-fatal: WAL failure is acceptable (device may not support it)
logger.debug(
"[CapacitorPlatformService] WAL mode failed, continuing with default:",
this._errMsg(walError),
);
journalMode = "default";
} }
// Only set synchronous when WAL is effective (invariant #4)
if (journalMode === "wal") { if (journalMode === "wal") {
// Only set NORMAL when WAL took effect
try { try {
await pragmaConn.run("PRAGMA synchronous = NORMAL;", []); await pragmaConn.run("PRAGMA synchronous = NORMAL;", []);
} catch { } catch (syncError) {
// Non-fatal if synchronous fails // Non-fatal if synchronous fails (may be in transaction)
logger.debug(
"[CapacitorPlatformService] synchronous=NORMAL failed (non-critical):",
this._errMsg(syncError),
);
} }
} }
} else { } else {
@@ -549,6 +577,22 @@ export class CapacitorPlatformService
* *
* v3.3.3: Fixed busy_timeout PRAGMA to use query() instead of run() to avoid * v3.3.3: Fixed busy_timeout PRAGMA to use query() instead of run() to avoid
* "error code 100: another row available" on some Capacitor SQLite builds. * "error code 100: another row available" on some Capacitor SQLite builds.
*
* INVARIANTS (v3.3.3+ - must not regress):
* 1. foreign_keys: Always set via run() (no return value)
* 2. journal_mode: Always set via query() (returns mode as row)
* - On iOS/web: If WAL refused, log + continue (journalMode = "unknown")
* - Never throw on WAL failure (device may not support it)
* 3. busy_timeout: Always set via query() first, fallback to run() if needed
* - Swallow benign "another row available" errors
* 4. synchronous: SKIPPED in fallback path (often fails in transaction)
* - Only set in temp path when journal_mode === 'wal'
* 5. MUST set this.db before returning (even on error, if conn exists)
*
* iOS Compatibility:
* - WAL may be refused on read-only bundles or protected file modes
* - If journal_mode !== 'wal', synchronous is not set (as documented)
* - All PRAGMA failures are non-fatal (log + continue)
*/ */
private async _applyPragmasOnExistingConnection(): Promise<{ private async _applyPragmasOnExistingConnection(): Promise<{
journalMode: string; journalMode: string;
@@ -628,20 +672,36 @@ export class CapacitorPlatformService
// Ignore errors // Ignore errors
} }
// journal_mode via query (safe even if txn-wrapped) // journal_mode via query (safe even if txn-wrapped) - invariant #2
if (platform !== "web") { if (platform !== "web") {
try { try {
const res = await conn.query("PRAGMA journal_mode = WAL;"); const res = await conn.query("PRAGMA journal_mode = WAL;");
const row = res.values?.[0] as Record<string, unknown> | undefined; const row = res.values?.[0] as Record<string, unknown> | undefined;
const key = row ? Object.keys(row)[0] : undefined; const key = row ? Object.keys(row)[0] : undefined;
journalMode = (key ? (row![key] as string) : undefined) ?? "unknown"; journalMode = (key ? (row![key] as string) : undefined) ?? "unknown";
} catch {
// leave as unknown // iOS compatibility: WAL may be refused on read-only bundles
// Log but don't throw - device may not support WAL
if (journalMode !== "wal") {
logger.debug(
`[CapacitorPlatformService] [PRAGMA] fallback: WAL mode not effective (got: ${journalMode}), continuing`,
);
}
} catch (walError) {
// Non-fatal: WAL failure is acceptable (device may not support it)
logger.debug(
"[CapacitorPlatformService] [PRAGMA] fallback: WAL mode failed, continuing:",
this._errMsg(walError),
);
journalMode = "default";
} }
} else { } else {
journalMode = "default"; journalMode = "default";
} }
// NOTE: synchronous is SKIPPED in fallback path (invariant #4)
// It often fails in transaction context, and temp path handles it when WAL is effective
// busy_timeout: use query() and ignore the row to avoid "another row available" (v3.3.3) // busy_timeout: use query() and ignore the row to avoid "another row available" (v3.3.3)
// v3.3.3: Fixed "error code 100: another row available" by using query() instead of run() // v3.3.3: Fixed "error code 100: another row available" by using query() instead of run()
// Some Capacitor SQLite builds return busy_timeout as a row, which causes run() to fail // Some Capacitor SQLite builds return busy_timeout as a row, which causes run() to fail
@@ -668,12 +728,12 @@ export class CapacitorPlatformService
} }
} }
// CRUCIAL: Set this.db before returning // CRUCIAL: Set this.db before returning (invariant #5)
this.db = conn; this.db = conn;
logger.log( logger.log(
`[CapacitorPlatformService] [PRAGMA] fallback: PRAGMAs applied; this.db set (journal_mode=${journalMode})`, `[CapacitorPlatformService] [PRAGMA] fallback: PRAGMAs applied; this.db set (journal_mode=${journalMode}, v3.3.3)`,
); );
logger.log("[CapacitorPlatformService] PRAGMA path: fallback"); logger.log("[CapacitorPlatformService] PRAGMA path: fallback (v3.3.3)");
return { journalMode, path: "fallback" as const }; return { journalMode, path: "fallback" as const };
} catch (e) { } catch (e) {