From 446cd694c608332fdb4635f9064a45a0e5773767 Mon Sep 17 00:00:00 2001 From: stevensJourney Date: Mon, 17 Feb 2025 16:07:39 +0200 Subject: [PATCH 1/2] handle mapper exceptions better --- .../Kotlin/KotlinPowerSyncDatabaseImpl.swift | 121 ++++++++------- Sources/PowerSync/Kotlin/SafeCastError.swift | 19 +++ .../Kotlin/TransactionCallback.swift | 35 +++++ .../PowerSync/Kotlin/wrapQueryCursor.swift | 51 +++++++ .../KotlinPowerSyncDatabaseImplTests.swift | 142 +++++++++++------- 5 files changed, 255 insertions(+), 113 deletions(-) create mode 100644 Sources/PowerSync/Kotlin/SafeCastError.swift create mode 100644 Sources/PowerSync/Kotlin/TransactionCallback.swift create mode 100644 Sources/PowerSync/Kotlin/wrapQueryCursor.swift diff --git a/Sources/PowerSync/Kotlin/KotlinPowerSyncDatabaseImpl.swift b/Sources/PowerSync/Kotlin/KotlinPowerSyncDatabaseImpl.swift index acc70ee..815b69c 100644 --- a/Sources/PowerSync/Kotlin/KotlinPowerSyncDatabaseImpl.swift +++ b/Sources/PowerSync/Kotlin/KotlinPowerSyncDatabaseImpl.swift @@ -4,16 +4,14 @@ import PowerSyncKotlin final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { private let kotlinDatabase: PowerSyncKotlin.PowerSyncDatabase - var currentStatus: SyncStatus { - get { kotlinDatabase.currentStatus } - } + var currentStatus: SyncStatus { kotlinDatabase.currentStatus } init( schema: Schema, dbFilename: String ) { let factory = PowerSyncKotlin.DatabaseDriverFactory() - self.kotlinDatabase = PowerSyncDatabase( + kotlinDatabase = PowerSyncDatabase( factory: factory, schema: KotlinAdapter.Schema.toKotlin(schema), dbFilename: dbFilename @@ -65,7 +63,7 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { } func execute(sql: String, parameters: [Any]?) async throws -> Int64 { - Int64(truncating: try await kotlinDatabase.execute(sql: sql, parameters: parameters)) + try Int64(truncating: await kotlinDatabase.execute(sql: sql, parameters: parameters)) } func get( @@ -73,11 +71,11 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: [Any]?, mapper: @escaping (SqlCursor) -> RowType ) async throws -> RowType { - try await kotlinDatabase.get( + try safeCast(await kotlinDatabase.get( sql: sql, parameters: parameters, mapper: mapper - ) as! RowType + ), to: RowType.self) } func get( @@ -85,13 +83,17 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: [Any]?, mapper: @escaping (SqlCursor) throws -> RowType ) async throws -> RowType { - try await kotlinDatabase.get( - sql: sql, - parameters: parameters, - mapper: { cursor in - try! mapper(cursor) - } - ) as! RowType + return try await wrapQueryCursorTyped( + mapper: mapper, + executor: { wrappedMapper in + try await self.kotlinDatabase.get( + sql: sql, + parameters: parameters, + mapper: wrappedMapper + ) + }, + resultType: RowType.self + ) } func getAll( @@ -99,11 +101,11 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: [Any]?, mapper: @escaping (SqlCursor) -> RowType ) async throws -> [RowType] { - try await kotlinDatabase.getAll( + try safeCast(await kotlinDatabase.getAll( sql: sql, parameters: parameters, mapper: mapper - ) as! [RowType] + ), to: [RowType].self) } func getAll( @@ -111,13 +113,17 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: [Any]?, mapper: @escaping (SqlCursor) throws -> RowType ) async throws -> [RowType] { - try await kotlinDatabase.getAll( - sql: sql, - parameters: parameters, - mapper: { cursor in - try! mapper(cursor) - } - ) as! [RowType] + try await wrapQueryCursorTyped( + mapper: mapper, + executor: { wrappedMapper in + try await self.kotlinDatabase.getAll( + sql: sql, + parameters: parameters, + mapper: wrappedMapper + ) + }, + resultType: [RowType].self + ) } func getOptional( @@ -125,11 +131,11 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: [Any]?, mapper: @escaping (SqlCursor) -> RowType ) async throws -> RowType? { - try await kotlinDatabase.getOptional( + try safeCast(await kotlinDatabase.getOptional( sql: sql, parameters: parameters, mapper: mapper - ) as! RowType? + ), to: RowType?.self) } func getOptional( @@ -137,13 +143,17 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: [Any]?, mapper: @escaping (SqlCursor) throws -> RowType ) async throws -> RowType? { - try await kotlinDatabase.getOptional( - sql: sql, - parameters: parameters, - mapper: { cursor in - try! mapper(cursor) - } - ) as! RowType? + try await wrapQueryCursorTyped( + mapper: mapper, + executor: { wrappedMapper in + try await self.kotlinDatabase.getOptional( + sql: sql, + parameters: parameters, + mapper: wrappedMapper + ) + }, + resultType: RowType?.self + ) } func watch( @@ -159,7 +169,7 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { parameters: parameters, mapper: mapper ) { - continuation.yield(values as! [RowType]) + try continuation.yield(safeCast(values, to: [RowType].self)) } continuation.finish() } catch { @@ -177,14 +187,23 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { AsyncThrowingStream { continuation in Task { do { - for await values in try self.kotlinDatabase.watch( + var mapperError: Error? + for try await values in try self.kotlinDatabase.watch( sql: sql, parameters: parameters, - mapper: { cursor in - try! mapper(cursor) - } + mapper: { cursor in do { + return try mapper(cursor) + } catch { + mapperError = error + // The value here does not matter. We will throw the exception later + // This is not ideal, this is only a workaround until we expose fine grained access to Kotlin SDK internals. + return nil as RowType? + } } ) { - continuation.yield(values as! [RowType]) + if mapperError != nil { + throw mapperError! + } + try continuation.yield(safeCast(values, to: [RowType].self)) } continuation.finish() } catch { @@ -195,33 +214,11 @@ final class KotlinPowerSyncDatabaseImpl: PowerSyncDatabaseProtocol { } public func writeTransaction(callback: @escaping (any PowerSyncTransaction) throws -> R) async throws -> R { - return try await kotlinDatabase.writeTransaction(callback: TransactionCallback(callback: callback)) as! R + return try safeCast(await kotlinDatabase.writeTransaction(callback: TransactionCallback(callback: callback)), to: R.self) } public func readTransaction(callback: @escaping (any PowerSyncTransaction) throws -> R) async throws -> R { - return try await kotlinDatabase.readTransaction(callback: TransactionCallback(callback: callback)) as! R - } -} - -class TransactionCallback: PowerSyncKotlin.ThrowableTransactionCallback { - let callback: (PowerSyncTransaction) throws -> R - - init(callback: @escaping (PowerSyncTransaction) throws -> R) { - self.callback = callback - } - - func execute(transaction: PowerSyncKotlin.PowerSyncTransaction) throws -> Any{ - do { - return try callback(transaction) - } catch let error { - return PowerSyncKotlin.PowerSyncException( - message: error.localizedDescription, - cause: PowerSyncKotlin.KotlinThrowable(message: error.localizedDescription) - ) - } + return try safeCast(await kotlinDatabase.readTransaction(callback: TransactionCallback(callback: callback)), to: R.self) } } -enum PowerSyncError: Error { - case invalidTransaction -} diff --git a/Sources/PowerSync/Kotlin/SafeCastError.swift b/Sources/PowerSync/Kotlin/SafeCastError.swift new file mode 100644 index 0000000..ccf736d --- /dev/null +++ b/Sources/PowerSync/Kotlin/SafeCastError.swift @@ -0,0 +1,19 @@ +enum SafeCastError: Error, CustomStringConvertible { + case typeMismatch(expected: Any.Type, actual: Any?) + + var description: String { + switch self { + case let .typeMismatch(expected, actual): + let actualType = actual.map { String(describing: type(of: $0)) } ?? "nil" + return "Type mismatch: Expected \(expected), but got \(actualType)." + } + } +} + +internal func safeCast(_ value: Any?, to type: T.Type) throws -> T { + if let castedValue = value as? T { + return castedValue + } else { + throw SafeCastError.typeMismatch(expected: type, actual: value) + } +} diff --git a/Sources/PowerSync/Kotlin/TransactionCallback.swift b/Sources/PowerSync/Kotlin/TransactionCallback.swift new file mode 100644 index 0000000..918c7a7 --- /dev/null +++ b/Sources/PowerSync/Kotlin/TransactionCallback.swift @@ -0,0 +1,35 @@ +import PowerSyncKotlin + +class TransactionCallback: PowerSyncKotlin.ThrowableTransactionCallback { + let callback: (PowerSyncTransaction) throws -> R + + init(callback: @escaping (PowerSyncTransaction) throws -> R) { + self.callback = callback + } + + // The Kotlin SDK does not gracefully handle exceptions thrown from Swift callbacks. + // If a Swift callback throws an exception, it results in a `BAD ACCESS` crash. + // + // To prevent this, we catch the exception and return it as a `PowerSyncException`, + // allowing Kotlin to propagate the error correctly. + // + // This approach is a workaround. Ideally, we should introduce an internal mechanism + // in the Kotlin SDK to handle errors from Swift more robustly. + // + // Currently, we wrap the public `PowerSyncDatabase` class in Kotlin, which limits our + // ability to handle exceptions cleanly. Instead, we should expose an internal implementation + // from a "core" package in Kotlin that provides better control over exception handling + // and other functionality—without modifying the public `PowerSyncDatabase` API to include + // Swift-specific logic. + func execute(transaction: PowerSyncKotlin.PowerSyncTransaction) throws -> Any { + do { + return try callback(transaction) + } catch { + return PowerSyncKotlin.PowerSyncException( + message: error.localizedDescription, + cause: PowerSyncKotlin.KotlinThrowable(message: error.localizedDescription) + ) + } + } +} + diff --git a/Sources/PowerSync/Kotlin/wrapQueryCursor.swift b/Sources/PowerSync/Kotlin/wrapQueryCursor.swift new file mode 100644 index 0000000..ad57f31 --- /dev/null +++ b/Sources/PowerSync/Kotlin/wrapQueryCursor.swift @@ -0,0 +1,51 @@ + +// The Kotlin SDK does not gracefully handle exceptions thrown from Swift callbacks. +// If a Swift callback throws an exception, it results in a `BAD ACCESS` crash. +// +// This approach is a workaround. Ideally, we should introduce an internal mechanism +// in the Kotlin SDK to handle errors from Swift more robustly. +// +// This hoists any exceptions thrown in a cursor mapper in order for the error to propagate correctly. +// +// Currently, we wrap the public `PowerSyncDatabase` class in Kotlin, which limits our +// ability to handle exceptions cleanly. Instead, we should expose an internal implementation +// from a "core" package in Kotlin that provides better control over exception handling +// and other functionality—without modifying the public `PowerSyncDatabase` API to include +// Swift-specific logic. +internal func wrapQueryCursor( + mapper: @escaping (SqlCursor) throws -> RowType, + // The Kotlin APIs return the results as Any, we can explicitly cast internally + executor: @escaping (_ wrappedMapper: @escaping (SqlCursor) -> RowType?) async throws -> ReturnType +) async throws -> ReturnType { + var mapperException: Error? + + // Wrapped version of the mapper that catches exceptions and sets `mapperException` + // In the case of an exception this will return an empty result. + let wrappedMapper: (SqlCursor) -> RowType? = { cursor in + do { + return try mapper(cursor) + } catch { + // Store the error in order to propagate it + mapperException = error + // Return nothing here. Kotlin should handle this as an empty object/row + return nil + } + } + + let executionResult = try await executor(wrappedMapper) + if mapperException != nil { + // Allow propagating the error + throw mapperException! + } + + return executionResult +} + +internal func wrapQueryCursorTyped( + mapper: @escaping (SqlCursor) throws -> RowType, + // The Kotlin APIs return the results as Any, we can explicitly cast internally + executor: @escaping (_ wrappedMapper: @escaping (SqlCursor) -> RowType?) async throws -> Any?, + resultType: ReturnType.Type +) async throws -> ReturnType { + return try safeCast(await wrapQueryCursor(mapper: mapper, executor: executor), to: resultType) +} diff --git a/Tests/PowerSyncTests/Kotlin/KotlinPowerSyncDatabaseImplTests.swift b/Tests/PowerSyncTests/Kotlin/KotlinPowerSyncDatabaseImplTests.swift index c929fed..ef2eaf1 100644 --- a/Tests/PowerSyncTests/Kotlin/KotlinPowerSyncDatabaseImplTests.swift +++ b/Tests/PowerSyncTests/Kotlin/KotlinPowerSyncDatabaseImplTests.swift @@ -1,5 +1,5 @@ -import XCTest @testable import PowerSync +import XCTest final class KotlinPowerSyncDatabaseImplTests: XCTestCase { private var database: KotlinPowerSyncDatabaseImpl! @@ -10,8 +10,8 @@ final class KotlinPowerSyncDatabaseImplTests: XCTestCase { schema = Schema(tables: [ Table(name: "users", columns: [ .text("name"), - .text("email") - ]) + .text("email"), + ]), ]) database = KotlinPowerSyncDatabaseImpl( @@ -36,9 +36,9 @@ final class KotlinPowerSyncDatabaseImplTests: XCTestCase { XCTFail("Expected an error to be thrown") } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: INSERT INTO usersfail (id, name, email) VALUES (?, ?, ?) -no such table: usersfail -""") + error while compiling: INSERT INTO usersfail (id, name, email) VALUES (?, ?, ?) + no such table: usersfail + """) } } @@ -52,10 +52,10 @@ no such table: usersfail sql: "SELECT id, name, email FROM users WHERE id = ?", parameters: ["1"] ) { cursor in - ( - try cursor.getString(name: "id"), - try cursor.getString(name: "name"), - try cursor.getString(name: "email") + try ( + cursor.getString(name: "id"), + cursor.getString(name: "name"), + cursor.getString(name: "email") ) } @@ -70,18 +70,18 @@ no such table: usersfail sql: "SELECT id, name, email FROM usersfail WHERE id = ?", parameters: ["1"] ) { cursor in - ( - try cursor.getString(name: "id"), - try cursor.getString(name: "name"), - try cursor.getString(name: "email") + try ( + cursor.getString(name: "id"), + cursor.getString(name: "name"), + cursor.getString(name: "email") ) } XCTFail("Expected an error to be thrown") } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: SELECT id, name, email FROM usersfail WHERE id = ? -no such table: usersfail -""") + error while compiling: SELECT id, name, email FROM usersfail WHERE id = ? + no such table: usersfail + """) } } @@ -116,18 +116,36 @@ no such table: usersfail sql: "SELECT id, name, email FROM usersfail WHERE id = ?", parameters: ["1"] ) { cursor in - ( - try cursor.getString(name: "id"), - try cursor.getString(name: "name"), - try cursor.getString(name: "email") + try ( + cursor.getString(name: "id"), + cursor.getString(name: "name"), + cursor.getString(name: "email") ) } XCTFail("Expected an error to be thrown") } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: SELECT id, name, email FROM usersfail WHERE id = ? -no such table: usersfail -""") + error while compiling: SELECT id, name, email FROM usersfail WHERE id = ? + no such table: usersfail + """) + } + } + + func testMapperError() async throws { + _ = try await database.execute( + sql: "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", + parameters: ["1", "Test User", "test@example.com"] + ) + do { + let _ = try await database.getOptional( + sql: "SELECT id, name, email FROM users WHERE id = ?", + parameters: ["1"] + ) { _ throws in + throw NSError(domain: "TestError", code: 1, userInfo: [NSLocalizedDescriptionKey: "cursor error"]) + } + XCTFail("Expected an error to be thrown") + } catch { + XCTAssertEqual(error.localizedDescription, "cursor error") } } @@ -141,9 +159,9 @@ no such table: usersfail sql: "SELECT id, name FROM users ORDER BY id", parameters: nil ) { cursor in - ( - try cursor.getString(name: "id"), - try cursor.getString(name: "name") + try ( + cursor.getString(name: "id"), + cursor.getString(name: "name") ) } @@ -160,18 +178,18 @@ no such table: usersfail sql: "SELECT id, name, email FROM usersfail WHERE id = ?", parameters: ["1"] ) { cursor in - ( - try cursor.getString(name: "id"), - try cursor.getString(name: "name"), - try cursor.getString(name: "email") + try ( + cursor.getString(name: "id"), + cursor.getString(name: "name"), + cursor.getString(name: "email") ) } XCTFail("Expected an error to be thrown") } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: SELECT id, name, email FROM usersfail WHERE id = ? -no such table: usersfail -""") + error while compiling: SELECT id, name, email FROM usersfail WHERE id = ? + no such table: usersfail + """) } } @@ -247,10 +265,34 @@ no such table: usersfail XCTFail("Expected an error to be thrown") } catch { - XCTAssertEqual(error.localizedDescription, """ -error while compiling: EXPLAIN SELECT name FROM usersfail ORDER BY id -no such table: usersfail -""") + XCTAssertEqual(error.localizedDescription, """ + error while compiling: EXPLAIN SELECT name FROM usersfail ORDER BY id + no such table: usersfail + """) + } + } + + func testWatchMapperError() async throws { + do { + _ = try await database.execute( + sql: "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", + parameters: ["1", "User 1", "user1@example.com"] + ) + + let stream = try database.watch( + sql: "SELECT name FROM users ORDER BY id", + parameters: nil + ) { _ throws in throw NSError(domain: "TestError", code: 1, userInfo: [NSLocalizedDescriptionKey: "cursor error"]) } + + // Actually consume the stream to trigger the error + for try await _ in stream { + XCTFail("Should not receive any values") + } + + XCTFail("Expected an error to be thrown") + } catch { + print(error.localizedDescription) + XCTAssertEqual(error.localizedDescription, "cursor error") } } @@ -267,7 +309,6 @@ no such table: usersfail ) } - let result = try await database.get( sql: "SELECT COUNT(*) FROM users", parameters: [] @@ -282,7 +323,7 @@ no such table: usersfail let loopCount = 100 _ = try await database.writeTransaction { transaction in - for i in 1...loopCount { + for i in 1 ... loopCount { _ = try transaction.execute( sql: "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", parameters: [String(i), "Test User \(i)", "test\(i)@example.com"] @@ -290,7 +331,7 @@ no such table: usersfail _ = try transaction.execute( sql: "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", - parameters: [String(i*10000), "Test User \(i)-2", "test\(i)-2@example.com"] + parameters: [String(i * 10000), "Test User \(i)-2", "test\(i)-2@example.com"] ) } } @@ -315,9 +356,9 @@ no such table: usersfail } } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: INSERT INTO usersfail (id, name, email) VALUES (?, ?, ?) -no such table: usersfail -""") + error while compiling: INSERT INTO usersfail (id, name, email) VALUES (?, ?, ?) + no such table: usersfail + """) } } @@ -336,9 +377,9 @@ no such table: usersfail } } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: INSERT INTO usersfail (id, name, email) VALUES (?, ?, ?) -no such table: usersfail -""") + error while compiling: INSERT INTO usersfail (id, name, email) VALUES (?, ?, ?) + no such table: usersfail + """) } let result = try await database.getOptional( @@ -356,7 +397,6 @@ no such table: usersfail parameters: ["1", "Test User", "test@example.com"] ) - _ = try await database.readTransaction { transaction in let result = try transaction.get( sql: "SELECT COUNT(*) FROM users", @@ -381,9 +421,9 @@ no such table: usersfail } } catch { XCTAssertEqual(error.localizedDescription, """ -error while compiling: SELECT COUNT(*) FROM usersfail -no such table: usersfail -""") + error while compiling: SELECT COUNT(*) FROM usersfail + no such table: usersfail + """) } } } From 5ff1852d031a40d18b229f1fcd206039bf0672d1 Mon Sep 17 00:00:00 2001 From: stevensJourney Date: Tue, 18 Feb 2025 15:01:59 +0200 Subject: [PATCH 2/2] updated changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dedf40..fb66b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 1.0.0-Beta.7 + +* Fixed an issue where throwing exceptions in the query `mapper` could cause a runtime crash. +* Internally improved type casting. + ## 1.0.0-Beta.6 * BREAKING CHANGE: `watch` queries are now throwable and therefore will need to be accompanied by a `try` e.g.