Make SQLite setup failures reportable and recoverable on the legacy (non-JSI) bridge

CHANGELOG-Unreleased.md

@@ -11,6 +11,8 @@
 - [LokiJS] Multitab sync issue fix
 - [Android] Added linker flag for building with 16kB page alignment
 - [TS] make catchError visible to typescript
+- [SQLite] [iOS] Database setup failures on the legacy (non-JSI) bridge (database cannot be opened, schema setup or migration fails) now reject the promise and reach `onSetUpError` instead of raising an `NSException` that crashes the app
+- [SQLite] [iOS] [Android] After a failed schema setup or migration on the legacy (non-JSI) bridge, operations queued during setup are rejected with the setup error and the driver stays connected, so the app can recover with `database.unsafeResetDatabase()` instead of every call hanging (iOS) or being rejected with "No driver with tag N available" (Android). The Node.js SQLite bridge (`sqlite-node`) behaves the same way
 
 ### Performance
 

native/android/src/main/java/com/nozbe/watermelondb/Connection.java

@@ -17,12 +17,18 @@ public Waiting(ArrayList<Runnable> queueInWaiting) {
         }
     }
 
+    // Setup failed and no driver is available - every operation is rejected with the setup error
+    public static class Failed extends Connection {
+        public final Exception error;
+        public Failed(Exception error) {
+            this.error = error;
+        }
+    }
+
     public ArrayList<Runnable> getQueue() {
-        if (this instanceof Connected) {
-            return new ArrayList<>();
-        } else if (this instanceof Waiting) {
+        if (this instanceof Waiting) {
             return ((Waiting) this).queueInWaiting;
         }
-        return null;
+        return new ArrayList<>();
     }
 }

native/android/src/main/java/com/nozbe/watermelondb/WMDatabase.java

@@ -149,9 +149,9 @@ public String getFromLocalStorage(String key) {
     private ArrayList<String> getAllTables() {
         ArrayList<String> allTables = new ArrayList<>();
         try (Cursor cursor = rawQuery(Queries.select_tables)) {
-            cursor.moveToFirst();
+            // NOTE: The cursor is empty if everything was destroyed but the schema could not be set up
             int nameIndex = cursor.getColumnIndex("name");
-            if (nameIndex > -1) {
+            if (nameIndex > -1 && cursor.moveToFirst()) {
                 do {
                     allTables.add(cursor.getString(nameIndex));
                 } while (cursor.moveToNext());

native/android/src/main/java/com/nozbe/watermelondb/WMDatabaseBridge.java

@@ -68,16 +68,25 @@ public void initialize(final Integer tag, final String databaseName, final int s
 
     @ReactMethod
     public void setUpWithSchema(final Integer tag, final String databaseName, final String schema, final int schemaVersion, final boolean unsafeNativeReuse, final Promise promise) {
-        connectDriver(tag, new WMDatabaseDriver(reactContext, databaseName, new Schema(schemaVersion, schema), unsafeNativeReuse), promise);
+        WMDatabaseDriver driver = null;
+        try {
+            driver = new WMDatabaseDriver(reactContext, databaseName, unsafeNativeReuse);
+            driver.unsafeResetDatabase(new Schema(schemaVersion, schema));
+            connectDriver(tag, driver, promise);
+        } catch (Exception e) {
+            failSetUp(tag, driver, e, promise);
+        }
     }
 
     @ReactMethod
     public void setUpWithMigrations(final Integer tag, final String databaseName, final String migrations, final int fromVersion, final int toVersion, final boolean unsafeNativeReuse, final Promise promise) {
+        WMDatabaseDriver driver = null;
         try {
-            connectDriver(tag, new WMDatabaseDriver(reactContext, databaseName, new MigrationSet(fromVersion, toVersion, migrations), unsafeNativeReuse), promise);
+            driver = new WMDatabaseDriver(reactContext, databaseName, unsafeNativeReuse);
+            driver.migrate(new MigrationSet(fromVersion, toVersion, migrations));
+            connectDriver(tag, driver, promise);
         } catch (Exception e) {
-            disconnectDriver(tag);
-            promise.reject(e);
+            failSetUp(tag, driver, e, promise);
         }
     }
 
@@ -142,6 +151,8 @@ public WritableArray unsafeGetLocalSynchronously(int tag, String key) {
                 return result;
             } else if (connection instanceof Connection.Waiting) {
                 throw new Exception("Waiting connection unexpected for unsafeGetLocalSynchronously");
+            } else if (connection instanceof Connection.Failed) {
+                throw ((Connection.Failed) connection).error;
             }
         } catch (Exception e) {
             WritableArray result = Arguments.createArray();
@@ -184,6 +195,8 @@ private void withDriver(final int tag, final Promise promise, final ParamFunctio
                 // try again when driver is ready
                 connection.getQueue().add(() -> withDriver(tag, promise, function, functionName));
                 connections.put(tag, new Connection.Waiting(connection.getQueue()));
+            } else if (connection instanceof Connection.Failed) {
+                promise.reject(functionName, ((Connection.Failed) connection).error);
             }
         } catch (Exception e) {
             promise.reject(functionName, e);
@@ -203,14 +216,23 @@ private void connectDriver(int connectionTag, WMDatabaseDriver driver, Promise p
         promise.resolve(true);
     }
 
-    private void disconnectDriver(int connectionTag) {
+    // Rejects the operations queued while waiting for setup with the setup error, then keeps the
+    // driver connected if the database could at least be opened.
+    // NOTE: Setup runs in a transaction, so after a failed setup the database is still consistent.
+    // Keeping the driver lets JS recover (e.g. by calling unsafeResetDatabase) instead of every
+    // subsequent call being rejected with "No driver with tag N available".
+    private void failSetUp(int connectionTag, WMDatabaseDriver driver, Exception error, Promise promise) {
         List<Runnable> queue = getQueue(connectionTag);
-
-        connections.remove(connectionTag);
+        connections.put(connectionTag, new Connection.Failed(error));
 
         for (Runnable operation : queue) {
             operation.run();
         }
+
+        if (driver != null) {
+            connections.put(connectionTag, new Connection.Connected(driver));
+        }
+        promise.reject(error);
     }
 
     @ReactMethod

native/android/src/main/java/com/nozbe/watermelondb/WMDatabaseDriver.java

@@ -206,7 +206,7 @@ public void close() {
         database.close();
     }
 
-    private void migrate(MigrationSet migrations) {
+    void migrate(MigrationSet migrations) {
         int databaseVersion = database.getUserVersion();
         if (databaseVersion != migrations.from) {
             throw new IllegalArgumentException("Incompatible migration set applied. " +

native/ios/WatermelonDB/objc/WMDatabase.h

@@ -5,11 +5,12 @@ NS_ASSUME_NONNULL_BEGIN
 @interface WMDatabase : NSObject
 
 @property (readwrite, strong, nonatomic) FMDatabase *fmdb;
-@property (readwrite, nonatomic) long userVersion;
+@property (readonly, nonatomic) long userVersion;
 
 #pragma mark - Initialization
 
-+ (instancetype) databaseWithPath:(NSString *)path;
+/// Returns nil (and sets errorPtr) if the database could not be opened
++ (nullable instancetype) databaseWithPath:(NSString *)path error:(NSError **)errorPtr;
 
 #pragma mark - Executing queries
 
@@ -22,6 +23,7 @@ NS_ASSUME_NONNULL_BEGIN
 #pragma mark - Other database functions
 
 - (BOOL) inTransaction:(BOOL (^)(NSError**))transactionBlock error:(NSError**)errorPtr;
+- (BOOL) setUserVersion:(long)userVersion error:(NSError **)errorPtr;
 - (BOOL) unsafeDestroyEverything:(NSError**)errorPtr;
 
 @end

native/ios/WatermelonDB/objc/WMDatabase.m

@@ -5,26 +5,32 @@ @implementation WMDatabase {
     NSString *_path;
 }
 
-- (instancetype) initWithPath:(NSString *)path
+- (nullable instancetype) initWithPath:(NSString *)path error:(NSError **)errorPtr
 {
     if (self = [super init]) {
         _path = path;
         _fmdb = [FMDatabase databaseWithPath:path];
-        [self open];
+        if (![self open:errorPtr]) {
+            return nil;
+        }
     }
 
     return self;
 }
 
-+ (instancetype) databaseWithPath:(NSString *)path
++ (nullable instancetype) databaseWithPath:(NSString *)path error:(NSError **)errorPtr
 {
-    return [[self alloc] initWithPath:path];
+    return [[self alloc] initWithPath:path error:errorPtr];
 }
 
-- (void) open
+- (BOOL) open:(NSError **)errorPtr
 {
+    // NOTE: Must not raise an NSException - exceptions thrown from async bridge methods crash the app
     if (![_fmdb open]) {
-        [NSException raise:@"OpenFailed" format:@"Failed to open the database: %@", _fmdb.lastErrorMessage];
+        *errorPtr = [NSError errorWithDomain:@"WMDatabase" code:0 userInfo:@{
+            NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Failed to open the database: %@", _fmdb.lastErrorMessage]
+        }];
+        return NO;
     }
 
     // TODO: Experiment with WAL
@@ -33,6 +39,7 @@ - (void) open
 
     // TODO: Configurable logger
     NSLog(@"Opened database at %@", _path);
+    return YES;
 }
 
 #pragma mark - Executing queries
@@ -116,13 +123,15 @@ - (long) userVersion
     return [result longForColumnIndex:0];
 }
 
-- (void) setUserVersion:(long)userVersion
+- (BOOL) setUserVersion:(long)userVersion error:(NSError **)errorPtr
 {
     NSString *sql = [NSString stringWithFormat:@"pragma user_version = %li", userVersion];
-    BOOL result = [_fmdb executeUpdate:sql];
-    if (!result) {
-        [NSException raise:@"SetUserVersionFailed" format:@"Failed to set user version: %@", _fmdb.lastErrorMessage];
+    if (![_fmdb executeUpdate:sql]) {
+        *errorPtr = _fmdb.lastError;
+        return NO;
     }
+
+    return YES;
 }
 
 - (BOOL) unsafeDestroyEverything:(NSError**)errorPtr
@@ -176,8 +185,7 @@ - (BOOL) unsafeDestroyEverything:(NSError**)errorPtr
         [manager removeItemAtPath:[NSString stringWithFormat:@"%@-shm", _path] error:nil];
 
         // reopen database
-        [self open];
-        return YES;
+        return [self open:errorPtr];
     }
 }
 

native/ios/WatermelonDB/objc/WMDatabaseBridge.m

@@ -53,7 +53,11 @@ + (BOOL) requiresMainQueueSetup
         return reject(@"db.initialize.error", [NSString stringWithFormat:@"A driver with tag %@ is already set up", tag], nil);
     }
 
-    WMDatabaseDriver *driver = [WMDatabaseDriver driverWithName:name];
+    NSError *error;
+    WMDatabaseDriver *driver = [WMDatabaseDriver driverWithName:name error:&error];
+    if (!driver) {
+        return reject(@"db.initialize.error", error.localizedDescription, error);
+    }
     WMDatabaseCompatibility compatibility = [driver isCompatibleWithSchemaVersion:[version integerValue]];
 
     if (compatibility == WMDatabaseCompatibilityCompatible) {
@@ -67,7 +71,7 @@ + (BOOL) requiresMainQueueSetup
         return resolve(@{@"code": @"migrations_needed", @"databaseVersion": @(driver.schemaVersion)});
     }
 
-    [NSException raise:@"BadArgument" format:@"Unexpected WMDatabaseCompatibility"];
+    return reject(@"db.initialize.error", @"Unexpected WMDatabaseCompatibility", nil);
 }
 
 BRIDGE_METHOD(setUpWithSchema,
@@ -76,10 +80,16 @@ + (BOOL) requiresMainQueueSetup
     schemaVersion:(nonnull NSNumber *)version
 )
 {
-    WMDatabaseDriver *driver = [WMDatabaseDriver driverWithName:name];
-    [driver setUpWithSchema:schema schemaVersion:[version integerValue]];
-    [self connectDriverAsync:tag driver:driver];
-    return resolve(@YES);
+    NSError *error;
+    WMDatabaseDriver *driver = [WMDatabaseDriver driverWithName:name error:&error];
+
+    if (driver && [driver setUpWithSchema:schema schemaVersion:[version integerValue] error:&error]) {
+        [self connectDriverAsync:tag driver:driver];
+        return resolve(@YES);
+    } else {
+        [self connectDriverAsync:tag driver:driver setUpError:error];
+        return reject(@"db.setUpWithSchema.error", error.localizedDescription, error);
+    }
 }
 
 BRIDGE_METHOD(setUpWithMigrations,
@@ -89,14 +99,14 @@ + (BOOL) requiresMainQueueSetup
     toVersion:(nonnull NSNumber *)toVersion
 )
 {
-    WMDatabaseDriver *driver = [WMDatabaseDriver driverWithName:name];
     NSError *error;
+    WMDatabaseDriver *driver = [WMDatabaseDriver driverWithName:name error:&error];
 
-    if ([driver setUpWithMigrations:migrationSQL fromVersion:[fromVersion integerValue] toVersion:[toVersion integerValue] error:&error]) {
+    if (driver && [driver setUpWithMigrations:migrationSQL fromVersion:[fromVersion integerValue] toVersion:[toVersion integerValue] error:&error]) {
         [self connectDriverAsync:tag driver:driver];
         return resolve(@YES);
     } else {
-        [self disconnectDriver:tag];
+        [self connectDriverAsync:tag driver:driver setUpError:error];
         return reject(@"db.setUpWithMigrations.error", error.localizedDescription, error);
     }
 }
@@ -259,20 +269,31 @@ + (BOOL) requiresMainQueueSetup
 - (void) connectDriverAsync:(nonnull NSNumber *)tag
                      driver:(WMDatabaseDriver *)driver
 {
-    _connections[tag] = driver;
-    NSArray *queuedOperations = _queue[tag];
-    [_queue removeObjectForKey:tag];
-
-    for (void (^action)(void) in queuedOperations) {
-        action();
-    }
+    [self connectDriverAsync:tag driver:driver setUpError:nil];
 }
 
-- (void) disconnectDriver:(nonnull NSNumber *)tag
+// Connects the driver (if setup produced one) and settles the operations queued while waiting for
+// it: they are retried if setup succeeded, and rejected with the setup error otherwise.
+// NOTE: Setup runs in a transaction, so after a failed setup the database is still consistent and
+// the driver stays connected. This lets JS recover (e.g. by calling unsafeResetDatabase) instead
+// of every subsequent call hanging on a tag that has neither a driver nor a queue.
+- (void) connectDriverAsync:(nonnull NSNumber *)tag
+                     driver:(nullable WMDatabaseDriver *)driver
+                 setUpError:(nullable NSError *)setUpError
 {
-    [_connections removeObjectForKey:tag];
-    // NOTE: In Swift, queued operations are executed, which seems wrong
+    if (driver && driver.db.fmdb.isOpen) {
+        _connections[tag] = driver;
+    } else {
+        // NOTE: A driver whose database could not be (re)opened is of no use - every operation
+        // on it would fail, so treat it like a missing driver
+        [_connections removeObjectForKey:tag];
+    }
+    NSArray *queuedOperations = _queue[tag];
     [_queue removeObjectForKey:tag];
+
+    for (void (^action)(NSError * _Nullable) in queuedOperations) {
+        action(setUpError);
+    }
 }
 
 - (void) withDriver:(nonnull NSNumber *)tag
@@ -293,11 +314,20 @@ - (void) withDriver:(nonnull NSNumber *)tag
             return resolve(result);
         }
     } else {
-        NSLog(@"Operation for driver %@ enqueued", tag);
         NSMutableArray *queuedOperations = _queue[tag];
+        if (!queuedOperations) {
+            // NOTE: Without this, the operation would be silently dropped and its promise never settled
+            NSString *errorName = [NSString stringWithFormat:@"db.%s.error", methodName];
+            return reject(errorName, [NSString stringWithFormat:@"No driver with tag %@ available", tag], nil);
+        }
+        NSLog(@"Operation for driver %@ enqueued", tag);
 
-        // try again when driver is ready
-        void (^queueOperation)(void) = ^() {
+        // try again when driver is ready, or fail if setting it up failed
+        void (^queueOperation)(NSError * _Nullable) = ^(NSError * _Nullable setUpError) {
+            if (setUpError) {
+                NSString *errorName = [NSString stringWithFormat:@"db.%s.error", methodName];
+                return reject(errorName, setUpError.localizedDescription, setUpError);
+            }
             [self withDriver:tag resolve:resolve reject:reject methodName:methodName action:action];
         };
 

native/ios/WatermelonDB/objc/WMDatabaseDriver.h

@@ -15,13 +15,14 @@ typedef NS_ENUM(NSInteger, WMDatabaseCompatibility) {
 
 #pragma mark - Initialization
 
-+ (instancetype) driverWithName:(NSString *)dbName;
+/// Returns nil (and sets errorPtr) if the database could not be opened
++ (nullable instancetype) driverWithName:(NSString *)dbName error:(NSError **)errorPtr;
 
 #pragma mark - Setup
 
 - (long) schemaVersion;
 - (WMDatabaseCompatibility) isCompatibleWithSchemaVersion:(long)version;
-- (void) setUpWithSchema:(NSString *)sql schemaVersion:(long)version;
+- (BOOL) setUpWithSchema:(NSString *)sql schemaVersion:(long)version error:(NSError **)errorPtr;
 - (BOOL) setUpWithMigrations:(NSString *)sql fromVersion:(long)fromVersion toVersion:(long)toVersion error:(NSError **)errorPtr;
 
 #pragma mark - Database functions