From d36a7f58c0db4cae462ba44f0c8c97a67cef1e64 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 27 Jan 2023 11:37:37 -0600 Subject: [PATCH] Handle SQLite corruption during database migration, take 2 I recommend reviewing this with whitespace changes disabled. If a database migration fails, we want to do two things: 1. Check if it failed due to database corruption. If so, flag the database as corrupted to enable recovery. 2. Crash. We were only doing the second part because each individual migration would crash instead of throwing the error. The logic was effectively this: do { for migration in migrations { do { try migration.run() } catch { crash() } } } catch { // This `catch` would never happen because a failed migration // would crash, not throw. flagDatabaseCorruptionIfNecessary(error) crash() } Now it's this: do { for migration in migrations { try migration.run() } } catch { flagDatabaseCorruptionIfNecessary(error) crash() } See 3892b9ead6e4588215bab5ed05f799835688a09b, which was my first attempt at doing this. Also see for people experiencing this issue. --- .../Storage/Database/GRDBSchemaMigrator.swift | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift b/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift index a1d7bda506..dfb501509d 100644 --- a/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift +++ b/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift @@ -363,21 +363,17 @@ public class GRDBSchemaMigrator: NSObject { } Logger.info("Running migration: \(identifier)") - do { - let transaction = GRDBWriteTransaction(database: database) - let result = try migrate(transaction) - switch result { - case .success: - let timeElapsed = CACurrentMediaTime() - startTime - let formattedTime = String(format: "%0.2fms", timeElapsed * 1000) - Logger.info("Migration completed: \(identifier), duration: \(formattedTime)") - case .failure(let error): - owsFail("Migration \(identifier) failed with returned error: \(error)") - } - transaction.finalizeTransaction() - } catch { - owsFail("Migration \(identifier) failed with thrown error: \(error)") + let transaction = GRDBWriteTransaction(database: database) + let result = try migrate(transaction) + switch result { + case .success: + let timeElapsed = CACurrentMediaTime() - startTime + let formattedTime = String(format: "%0.2fms", timeElapsed * 1000) + Logger.info("Migration completed: \(identifier), duration: \(formattedTime)") + case .failure(let error): + throw error } + transaction.finalizeTransaction() } }