Skip to content

Conversation

kpgalligan
Copy link
Contributor

Exceptions from sql calls are now wrapped with a PowerSyncException, and explicitly thrown to Swift.

Exceptions now look more like this:

PowerSyncTests/KotlinPowerSyncDatabaseImplTests.swift:64: Fatal error: 'try!' expression unexpectedly raised an error: Error Domain=KotlinException Code=0 "Column 'name2' not found" UserInfo={NSLocalizedDescription=Column 'name2' not found, KotlinException=kotlin.IllegalArgumentException: Column 'name2' not found, KotlinExceptionOrigin=}

Also, the stack trace was being printed in Kotlin by the sqliter logger. A new logger instance is passed in that doesn't print anything inside Kotlin. Summary, you should see this anymore:

no such table: user_profilesincorrect | error code SQLITE_ERROR
co.touchlab.sqliter.interop.SQLiteExceptionErrorCode: error while compiling: EXPLAIN SELECT * FROM user_profilesincorrect
no such table: user_profilesincorrect
    at 0   AirAppsCloudExample.debug.dylib     0x104f19eb3        kfun:co.touchlab.sqliter.interop.SQLiteExceptionErrorCode#<init>(kotlin.String;co.touchlab.sqliter.interop.SqliteDatabaseConfig;kotlin.Int){} + 111
    at 1   AirAppsCloudExample.debug.dylib     0x104f228c3        kfun:co.touchlab.sqliter.native.NativeDatabaseConnection#createStatement(kotlin.String){}co.touchlab.sqliter.Statement + 2867
    at 2   AirAppsCloudExample.debug.dylib     0x10506e73f        kfun:co.touchlab.sqliter.DatabaseConnection#createStatement(kotlin.String){}co.touchlab.sqliter.Statement-trampoline + 203
    at 3   AirAppsCloudExample.debug.dylib     0x104f133af        kfun:co.touchlab.sqliter.concurrency.ConcurrentDatabaseConnection#createStatement(kotlin.String){}co.touchlab.sqliter.Statement + 127
    at 4   AirAppsCloudExample.debug.dylib     0x10506e73f        kfun:co.touchlab.sqliter.DatabaseConnection#createStatement(kotlin.String){}co.touchlab.sqliter.Statement-trampoline + 203
    at 5   AirAppsCloudExample.debug.dylib     0x104f29cef        kfun:app.cash.sqldelight.driver.native.ConnectionWrapper.$accessStatement$lambda$0$FUNCTION_REFERENCE$16.invoke#internal + 251
    at 6   AirAppsCloudExample.debug.dylib     0x10505e257        kfun:kotlin.Function1#invoke(1:0){}1:1-trampoline + 203
    at 7   AirAppsCloudExample.debug.dylib     0x104f2afcf        kfun:app.cash.sqldelight.driver.native.Pool#access(kotlin.Function1<1:0,0:0>){0§<kotlin.Any?>}0:0 + 143
    at 8   AirAppsCloudExample.debug.dylib     0x104f286a3        kfun:app.cash.sqldelight.driver.native.NativeSqliteDriver#accessConnection(kotlin.Boolean;kotlin.Function1<app.cash.sqldelight.driver.native.ThreadConnection,0:0>){0§<kotlin.Any?>}0:0 + 211
    at 9   AirAppsCloudExample.debug.dylib     0x104f298eb        kfun:app.cash.sqldelight.driver.native.ConnectionWrapper.accessStatement#internal + 319
    at 10  AirAppsCloudExample.debug.dylib     0x104f29bcb       
Etc...

I saw @FunctionInterop.FileScopeConversion.Disabled was added to the extension methods in cursor, but then those methods were available from Swift, or their signature was odd. I moved those methods directly into SqlCursor instead of having them as extension methods. Should discuss the original issue and make sure there isn't an issue.

Accessing cursor by name, if a bad name is given, throws IllegalArgumentException. You should decide is that's OK, or if it should also be PowerSyncException.

@kpgalligan
Copy link
Contributor Author

Some more updates. Sql exceptions look like this:

/Users/kevingalligan/devel-clients/powersync-swift/Sources/PowerSync/Kotlin/KotlinPowerSyncDatabaseImpl.swift:76: error: -[PowerSyncTests.KotlinPowerSyncDatabaseImplTests testInsertAndGetByName] : failed: caught error: "Error Domain=KotlinException Code=0 "error while compiling: SELECT id, name, email FROM users2 WHERE id = ?
no such table: users2" UserInfo={NSLocalizedDescription=error while compiling: SELECT id, name, email FROM users2 WHERE id = ?
no such table: users2, KotlinException=com.powersync.PowerSyncException: error while compiling: SELECT id, name, email FROM users2 WHERE id = ?
no such table: users2, KotlinExceptionOrigin=}"

The no such table: users2 lines are being duplicated, which may be due to passing the wrapped exception in as a cause to the constructor to PowerSyncException. If so, I'm not sure we can eliminate that without skipping that, or some kind of custom process to convert the exception to something iOS would prefer. I'd be reluctant to do that, for clients that aren't iOS, as they might get some use out of the cause exception, but it's easy enough to try and change.

For the the Error 2/fetchCredentials from the Slack post, nothing in Kotlin was actually throwing any exceptions directly. All of the try calls in Swift were due to the suspend functions. SKIE generates real async calls, as opposed to what the Kotlin compiler generates. That means SKIE suspend calls require try because they may be cancelled. However, they would only ever throw cancellations. To get other exceptions, they need to be explicitly added in a @Throws annotation. I've wrapped the Kotlin calls in the connector code to throw PowerSyncException. That should now throw something to Swift that can be properly caught and handled.

I don't currently have a full test setup for Supabase. As I was working on this today, I got an email saying my test project instance was paused. I could go in and try to re-spin that up, but if you've got something ready to test with catching Swift exceptions, I think it would be simpler to just try this updated client code with that and see if it works as expected.

One issue to note. com.powersync.connectors.PowerSyncBackendConnector#prefetchCredentials returns a Job instance. I honestly don't know off hand where a possible exception from that job would be thrown, but I'd have to guess it is wherever join or similar are called. I did not go into these parts of the API to lock down visibility, as I'm not 100% sure who would need access to these structures. They're essentially all public, which is not great if they don't need to be. If there are methods that don't need to be public, they should be internal. That would make the job of figuring out which methods needs @Throws easier, but also that might prevent exporting things you don't really need from Swift. Job is a heavily-used part of Coroutines, so I'd guess it's pretty conservative about what it exports from the Coroutines library, but I haven't walked the graph. If prefetchCredentials is something a public caller would want, it might be better to wrap Job with the minimal API a public caller would need.

@DominicGBauer
Copy link
Contributor

This can be closed because it is included in this PR #116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants