From bd40a7fb985b5f0d869e55e128c7cd8097beee48 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Thu, 2 Mar 2023 09:59:18 -0800 Subject: [PATCH] Graceful handling of readonly DB error --- app/main.ts | 20 ++++++++++++++++ ts/sql/Client.ts | 9 +++++-- ts/sql/errors.ts | 35 +++++++++++++++++++++------ ts/sql/main.ts | 61 ++++++++++++++++++++++++++++++++++-------------- 4 files changed, 98 insertions(+), 27 deletions(-) diff --git a/app/main.ts b/app/main.ts index b21b04de4..4f57ca828 100644 --- a/app/main.ts +++ b/app/main.ts @@ -1573,6 +1573,20 @@ const runSQLCorruptionHandler = async () => { await onDatabaseError(Errors.toLogFormat(error)); }; +const runSQLReadonlyHandler = async () => { + // This is a glorified event handler. Normally, this promise never resolves, + // but if there is a corruption error triggered by any query that we run + // against the database - the promise will resolve and we will call + // `onDatabaseError`. + const error = await sql.whenReadonly(); + + getLogger().error( + `Detected readonly sql database in main process: ${error.message}` + ); + + throw error; +}; + async function initializeSQL( userDataPath: string ): Promise<{ ok: true; error: undefined } | { ok: false; error: Error }> { @@ -1619,6 +1633,7 @@ async function initializeSQL( // Only if we've initialized things successfully do we set up the corruption handler drop(runSQLCorruptionHandler()); + drop(runSQLReadonlyHandler()); return { ok: true, error: undefined }; } @@ -1669,6 +1684,11 @@ ipc.on('database-error', (_event: Electron.Event, error: string) => { drop(onDatabaseError(error)); }); +ipc.on('database-readonly', (_event: Electron.Event, error: string) => { + // Just let global_errors.ts handle it + throw new Error(error); +}); + function loadPreferredSystemLocales(): Array { return getEnvironment() === Environment.Test ? ['en'] diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index aa929fe6d..78b436231 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -65,7 +65,7 @@ import type { StoredSignedPreKeyType, } from './Interface'; import Server from './Server'; -import { isCorruptionError } from './errors'; +import { parseSqliteError, SqliteErrorKind } from './errors'; import { MINUTE } from '../util/durations'; import { getMessageIdForLogging } from '../util/idForLogging'; @@ -309,13 +309,18 @@ function makeChannel(fnName: string) { // Ignoring this error TS2556: Expected 3 arguments, but got 0 or more. return await serverFn(...args); } catch (error) { - if (isCorruptionError(error)) { + const sqliteErrorKind = parseSqliteError(error); + if (sqliteErrorKind === SqliteErrorKind.Corrupted) { log.error( 'Detected sql corruption in renderer process. ' + `Restarting the application immediately. Error: ${error.message}` ); ipc?.send('database-error', error.stack); + } else if (sqliteErrorKind === SqliteErrorKind.Readonly) { + log.error(`Detected readonly sql database: ${error.message}`); + ipc?.send('database-readonly'); } + log.error( `Renderer SQL channel job (${fnName}) error ${error.message}` ); diff --git a/ts/sql/errors.ts b/ts/sql/errors.ts index 1666511bb..29d3a828a 100644 --- a/ts/sql/errors.ts +++ b/ts/sql/errors.ts @@ -1,11 +1,32 @@ // Copyright 2021 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -export function isCorruptionError(error?: Error): boolean { - return ( - error?.message?.includes('SQLITE_CORRUPT') || - error?.message?.includes('database disk image is malformed') || - error?.message?.includes('file is not a database') || - false - ); +export enum SqliteErrorKind { + Corrupted, + Readonly, + Unknown, +} + +export function parseSqliteError(error?: Error): SqliteErrorKind { + const message = error?.message; + if (!message) { + return SqliteErrorKind.Unknown; + } + + if ( + message.includes('SQLITE_CORRUPT') || + message.includes('database disk image is malformed') || + message.includes('file is not a database') + ) { + return SqliteErrorKind.Corrupted; + } + + if ( + message.includes('SQLITE_READONLY') || + message.includes('attempt to write a readonly database') + ) { + return SqliteErrorKind.Readonly; + } + + return SqliteErrorKind.Unknown; } diff --git a/ts/sql/main.ts b/ts/sql/main.ts index 994d0b54f..0772dd02d 100644 --- a/ts/sql/main.ts +++ b/ts/sql/main.ts @@ -9,7 +9,7 @@ import { app } from 'electron'; import { strictAssert } from '../util/assert'; import { explodePromise } from '../util/explodePromise'; import type { LoggerType } from '../types/Logging'; -import { isCorruptionError } from './errors'; +import { parseSqliteError, SqliteErrorKind } from './errors'; import type DB from './Server'; const MIN_TRACE_DURATION = 40; @@ -64,6 +64,11 @@ type PromisePair = { reject: (error: Error) => void; }; +type KnownErrorResolverType = Readonly<{ + kind: SqliteErrorKind; + resolve: (err: Error) => void; +}>; + export class MainSQL { private readonly worker: Worker; @@ -73,9 +78,8 @@ export class MainSQL { private readonly onExit: Promise; - // This promise is resolved when any of the queries that we run against the - // database reject with a corruption error (see `isCorruptionError`) - private readonly onCorruption: Promise; + // Promise resolve callbacks for corruption and readonly errors. + private errorResolvers = new Array(); private seq = 0; @@ -88,10 +92,6 @@ export class MainSQL { const scriptDir = join(app.getAppPath(), 'ts', 'sql', 'mainWorker.js'); this.worker = new Worker(scriptDir); - const { promise: onCorruption, resolve: resolveCorruption } = - explodePromise(); - this.onCorruption = onCorruption; - this.worker.on('message', (wrappedResponse: WrappedWorkerResponse) => { if (wrappedResponse.type === 'log') { const { level, args } = wrappedResponse; @@ -110,9 +110,7 @@ export class MainSQL { if (error) { const errorObj = new Error(error); - if (isCorruptionError(errorObj)) { - resolveCorruption(errorObj); - } + this.onError(errorObj); pair.reject(errorObj); } else { @@ -120,9 +118,9 @@ export class MainSQL { } }); - this.onExit = new Promise(resolve => { - this.worker.once('exit', resolve); - }); + const { promise: onExit, resolve: resolveOnExit } = explodePromise(); + this.onExit = onExit; + this.worker.once('exit', resolveOnExit); } public async initialize({ @@ -148,7 +146,15 @@ export class MainSQL { } public whenCorrupted(): Promise { - return this.onCorruption; + const { promise, resolve } = explodePromise(); + this.errorResolvers.push({ kind: SqliteErrorKind.Corrupted, resolve }); + return promise; + } + + public whenReadonly(): Promise { + const { promise, resolve } = explodePromise(); + this.errorResolvers.push({ kind: SqliteErrorKind.Readonly, resolve }); + return promise; } public async close(): Promise { @@ -199,9 +205,8 @@ export class MainSQL { const { seq } = this; this.seq += 1; - const result = new Promise((resolve, reject) => { - this.onResponse.set(seq, { resolve, reject }); - }); + const { promise: result, resolve, reject } = explodePromise(); + this.onResponse.set(seq, { resolve, reject }); const wrappedRequest: WrappedWorkerRequest = { seq, @@ -211,4 +216,24 @@ export class MainSQL { return result; } + + private onError(error: Error): void { + const errorKind = parseSqliteError(error); + if (errorKind === SqliteErrorKind.Unknown) { + return; + } + + const resolvers = new Array<(error: Error) => void>(); + this.errorResolvers = this.errorResolvers.filter(entry => { + if (entry.kind === errorKind) { + resolvers.push(entry.resolve); + return false; + } + return true; + }); + + for (const resolve of resolvers) { + resolve(error); + } + } }