Tighten SQLite transaction failures and tests

lyngio/src/jvmMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformJvm.kt

@@ -68,14 +68,19 @@ private class JdbcSqliteDatabaseBackend(
         try {
             connection.autoCommit = false
             val tx = JdbcSqliteTransactionBackend(core, connection)
-            return try {
+            val result = try {
-                val result = block(tx)
+                block(tx)
-                connection.commit()
-                result
             } catch (e: Throwable) {
-                rollbackQuietly(connection)
+                throw finishFailedTransaction(scope, core, e) {
-                throw e
+                    rollbackOrThrow(scope, core, connection)
+                }
             }
+            try {
+                connection.commit()
+            } catch (e: SQLException) {
+                throw mapSqlException(scope, core, e)
+            }
+            return result
         } catch (e: SQLException) {
             throw mapSqlException(scope, core, e)
         } finally {
@@ -180,14 +185,21 @@ private class JdbcSqliteTransactionBackend(
         } catch (e: SQLException) {
             throw mapSqlUsage(scope, core, "Nested transactions are not supported by this SQLite backend", e)
         }
-        return try {
+        val nested = JdbcSqliteTransactionBackend(core, connection)
-            val result = block(JdbcSqliteTransactionBackend(core, connection))
+        val result = try {
-            connection.releaseSavepoint(savepoint)
+            block(nested)
-            result
         } catch (e: Throwable) {
-            rollbackQuietly(connection, savepoint)
+            throw finishFailedTransaction(scope, core, e) {
-            throw e
+                rollbackToSavepointOrThrow(scope, core, connection, savepoint)
+                releaseSavepointOrThrow(scope, core, connection, savepoint)
+            }
         }
+        try {
+            connection.releaseSavepoint(savepoint)
+        } catch (e: SQLException) {
+            throw mapSqlException(scope, core, e)
+        }
+        return result
     }
 }
 
@@ -383,13 +395,70 @@ private fun sqlExecutionFailure(scope: ScopeFacade, core: SqliteCoreModule, mess
     )
 }
 
-private fun rollbackQuietly(connection: Connection, savepoint: java.sql.Savepoint? = null) {
+private fun rollbackOrThrow(scope: ScopeFacade, core: SqliteCoreModule, connection: Connection) {
     try {
-        if (savepoint == null) connection.rollback() else connection.rollback(savepoint)
+        connection.rollback()
-    } catch (_: SQLException) {
+    } catch (e: SQLException) {
+        throw mapSqlException(scope, core, e)
     }
 }
 
+private fun rollbackToSavepointOrThrow(
+    scope: ScopeFacade,
+    core: SqliteCoreModule,
+    connection: Connection,
+    savepoint: java.sql.Savepoint,
+) {
+    try {
+        connection.rollback(savepoint)
+    } catch (e: SQLException) {
+        throw mapSqlException(scope, core, e)
+    }
+}
+
+private fun releaseSavepointOrThrow(
+    scope: ScopeFacade,
+    core: SqliteCoreModule,
+    connection: Connection,
+    savepoint: java.sql.Savepoint,
+) {
+    try {
+        connection.releaseSavepoint(savepoint)
+    } catch (e: SQLException) {
+        throw mapSqlException(scope, core, e)
+    }
+}
+
+private inline fun finishFailedTransaction(
+    scope: ScopeFacade,
+    core: SqliteCoreModule,
+    failure: Throwable,
+    rollback: () -> Unit,
+): Throwable {
+    return try {
+        rollback()
+        failure
+    } catch (rollbackFailure: Throwable) {
+        if (isRollbackSignal(failure, core)) {
+            attachSecondaryFailure(rollbackFailure, failure)
+            rollbackFailure
+        } else {
+            attachSecondaryFailure(failure, rollbackFailure)
+            failure
+        }
+    }
+}
+
+private fun isRollbackSignal(failure: Throwable, core: SqliteCoreModule): Boolean {
+    val errorObject = (failure as? ExecutionError)?.errorObject ?: return false
+    return errorObject.isInstanceOf(core.rollbackException)
+}
+
+private fun attachSecondaryFailure(primary: Throwable, secondary: Throwable) {
+    if (primary === secondary) return
+    primary.addSuppressed(secondary)
+}
+
 private fun mapOpenException(scope: ScopeFacade, core: SqliteCoreModule, e: SQLException): Nothing {
     val message = e.message ?: "SQLite open failed"
     val lower = message.lowercase()

lyngio/src/jvmTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleTest.kt

@@ -411,6 +411,88 @@ class LyngSqliteModuleTest {
         assertEquals("Bool|Date|12:34:56|Bool", summary.value)
     }
 
+    @Test
+    fun testUnsupportedParameterTypeFailsWithSqlUsageException() = runTest {
+        val scope = Script.newScope()
+        val db = openMemoryDb(scope)
+
+        val error = assertFailsWith<ExecutionError> {
+            db.invokeInstanceMethod(
+                scope,
+                "transaction",
+                ObjExternCallable.fromBridge {
+                    val tx = requiredArg<Obj>(0)
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("create table sample(value text not null)")
+                    )
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("insert into sample(value) values(?)"),
+                        emptyMapObj()
+                    )
+                }
+            )
+        }
+
+        assertEquals("SqlUsageException", error.errorObject.objClass.className)
+        assertTrue(error.errorMessage.contains("Unsupported SQLite parameter type"), error.errorMessage)
+    }
+
+    @Test
+    fun testTimestampAndDatetimeRejectTimezoneBearingText() = runTest {
+        val scope = Script.newScope()
+        withTempDb(scope) { db ->
+            db.invokeInstanceMethod(
+                scope,
+                "transaction",
+                ObjExternCallable.fromBridge {
+                    val tx = requiredArg<Obj>(0)
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("create table sample(ts TIMESTAMP not null, dt DATETIME not null)")
+                    )
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("insert into sample(ts, dt) values(?, ?)"),
+                        ObjString("2024-05-06T07:08:09Z"),
+                        ObjString("2024-05-06T10:11:12+03:00")
+                    )
+                }
+            )
+
+            val timestampError = assertFailsWith<ExecutionError> {
+                db.invokeInstanceMethod(
+                    scope,
+                    "transaction",
+                    ObjExternCallable.fromBridge {
+                        val tx = requiredArg<Obj>(0)
+                        tx.invokeInstanceMethod(requireScope(), "select", ObjString("select ts from sample"))
+                    }
+                )
+            }
+            assertEquals("SqlExecutionException", timestampError.errorObject.objClass.className)
+            assertTrue(timestampError.errorMessage.contains("must not contain a timezone offset"), timestampError.errorMessage)
+
+            val datetimeError = assertFailsWith<ExecutionError> {
+                db.invokeInstanceMethod(
+                    scope,
+                    "transaction",
+                    ObjExternCallable.fromBridge {
+                        val tx = requiredArg<Obj>(0)
+                        tx.invokeInstanceMethod(requireScope(), "select", ObjString("select dt from sample"))
+                    }
+                )
+            }
+            assertEquals("SqlExecutionException", datetimeError.errorObject.objClass.className)
+            assertTrue(datetimeError.errorMessage.contains("must not contain a timezone offset"), datetimeError.errorMessage)
+        }
+    }
+
     @Test
     fun testReadOnlyOpenPreventsWrites() = runTest {
         val scope = Script.newScope()

lyngio/src/linuxTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleNativeTest.kt

@@ -309,6 +309,88 @@ class LyngSqliteModuleNativeTest {
         assertEquals("Bool|Date|12:34:56|Bool", summary.value)
     }
 
+    @Test
+    fun testUnsupportedParameterTypeFailsWithSqlUsageException() = runTest {
+        val scope = Script.newScope()
+        val db = openMemoryDb(scope)
+
+        val error = assertFailsWith<ExecutionError> {
+            db.invokeInstanceMethod(
+                scope,
+                "transaction",
+                ObjExternCallable.fromBridge {
+                    val tx = requiredArg<Obj>(0)
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("create table sample(value text not null)")
+                    )
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("insert into sample(value) values(?)"),
+                        emptyMapObj()
+                    )
+                }
+            )
+        }
+
+        assertEquals("SqlUsageException", error.errorObject.objClass.className)
+        assertTrue(error.errorMessage.contains("Unsupported SQLite parameter type"), error.errorMessage)
+    }
+
+    @Test
+    fun testTimestampAndDatetimeRejectTimezoneBearingText() = runTest {
+        val scope = Script.newScope()
+        withTempDb(scope) { db ->
+            db.invokeInstanceMethod(
+                scope,
+                "transaction",
+                ObjExternCallable.fromBridge {
+                    val tx = requiredArg<Obj>(0)
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("create table sample(ts TIMESTAMP not null, dt DATETIME not null)")
+                    )
+                    tx.invokeInstanceMethod(
+                        requireScope(),
+                        "execute",
+                        ObjString("insert into sample(ts, dt) values(?, ?)"),
+                        ObjString("2024-05-06T07:08:09Z"),
+                        ObjString("2024-05-06T10:11:12+03:00")
+                    )
+                }
+            )
+
+            val timestampError = assertFailsWith<ExecutionError> {
+                db.invokeInstanceMethod(
+                    scope,
+                    "transaction",
+                    ObjExternCallable.fromBridge {
+                        val tx = requiredArg<Obj>(0)
+                        tx.invokeInstanceMethod(requireScope(), "select", ObjString("select ts from sample"))
+                    }
+                )
+            }
+            assertEquals("SqlExecutionException", timestampError.errorObject.objClass.className)
+            assertTrue(timestampError.errorMessage.contains("must not contain a timezone offset"), timestampError.errorMessage)
+
+            val datetimeError = assertFailsWith<ExecutionError> {
+                db.invokeInstanceMethod(
+                    scope,
+                    "transaction",
+                    ObjExternCallable.fromBridge {
+                        val tx = requiredArg<Obj>(0)
+                        tx.invokeInstanceMethod(requireScope(), "select", ObjString("select dt from sample"))
+                    }
+                )
+            }
+            assertEquals("SqlExecutionException", datetimeError.errorObject.objClass.className)
+            assertTrue(datetimeError.errorMessage.contains("must not contain a timezone offset"), datetimeError.errorMessage)
+        }
+    }
+
     @Test
     fun testReadOnlyOpenPreventsWrites() = runTest {
         val scope = Script.newScope()

lyngio/src/nativeMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformNative.kt

@@ -119,14 +119,15 @@ private class NativeSqliteDatabaseBackend(
         try {
             handle.execUnit(scope, core, "begin")
             val tx = NativeSqliteTransactionBackend(core, handle, savepoints)
-            return try {
+            val result = try {
-                val result = block(tx)
+                block(tx)
-                handle.execUnit(scope, core, "commit")
-                result
             } catch (e: Throwable) {
-                handle.execUnitQuietly("rollback")
+                throw finishFailedTransaction(scope, core, e) {
-                throw e
+                    handle.execUnit(scope, core, "rollback")
+                }
             }
+            handle.execUnit(scope, core, "commit")
+            return result
         } finally {
             handle.close()
         }
@@ -149,15 +150,17 @@ private class NativeSqliteTransactionBackend(
     override suspend fun <T> transaction(scope: ScopeFacade, block: suspend (SqliteTransactionBackend) -> T): T {
         val savepoint = "lyng_sp_${savepoints.next()}"
         handle.execUnit(scope, core, "savepoint $savepoint")
-        return try {
+        val nested = NativeSqliteTransactionBackend(core, handle, savepoints)
-            val result = block(NativeSqliteTransactionBackend(core, handle, savepoints))
+        val result = try {
-            handle.execUnit(scope, core, "release savepoint $savepoint")
+            block(nested)
-            result
         } catch (e: Throwable) {
-            handle.execUnitQuietly("rollback to savepoint $savepoint")
+            throw finishFailedTransaction(scope, core, e) {
-            handle.execUnitQuietly("release savepoint $savepoint")
+                handle.execUnit(scope, core, "rollback to savepoint $savepoint")
-            throw e
+                handle.execUnit(scope, core, "release savepoint $savepoint")
+            }
         }
+        handle.execUnit(scope, core, "release savepoint $savepoint")
+        return result
     }
 }
 
@@ -229,17 +232,6 @@ private class NativeSqliteHandle(
         }
     }
 
-    fun execUnitQuietly(sql: String) {
-        memScoped {
-            val stmt = lyng_sqlite3_prepare(db, sql) ?: return@memScoped
-            try {
-                sqlite3_step(stmt)
-            } finally {
-                sqlite3_finalize(stmt)
-            }
-        }
-    }
-
     fun close() {
         sqlite3_close_v2(db)
     }
@@ -600,3 +592,33 @@ private fun sqlExecutionError(scope: ScopeFacade, core: SqliteCoreModule, messag
         message,
     )
 }
+
+private inline fun finishFailedTransaction(
+    scope: ScopeFacade,
+    core: SqliteCoreModule,
+    failure: Throwable,
+    rollback: () -> Unit,
+): Throwable {
+    return try {
+        rollback()
+        failure
+    } catch (rollbackFailure: Throwable) {
+        if (isRollbackSignal(failure, core)) {
+            attachSecondaryFailure(rollbackFailure, failure)
+            rollbackFailure
+        } else {
+            attachSecondaryFailure(failure, rollbackFailure)
+            failure
+        }
+    }
+}
+
+private fun isRollbackSignal(failure: Throwable, core: SqliteCoreModule): Boolean {
+    val errorObject = (failure as? ExecutionError)?.errorObject ?: return false
+    return errorObject.isInstanceOf(core.rollbackException)
+}
+
+private fun attachSecondaryFailure(primary: Throwable, secondary: Throwable) {
+    if (primary === secondary) return
+    primary.addSuppressed(secondary)
+}