From 6d340824e4aa5c080fda8812796f2fa6e336f4a6 Mon Sep 17 00:00:00 2001 From: sergeych Date: Wed, 15 Apr 2026 22:20:29 +0300 Subject: [PATCH] Tighten SQLite transaction failures and tests --- .../sergeych/lyng/io/db/sqlite/PlatformJvm.kt | 99 ++++++++++++++++--- .../lyng/io/db/sqlite/LyngSqliteModuleTest.kt | 82 +++++++++++++++ .../db/sqlite/LyngSqliteModuleNativeTest.kt | 82 +++++++++++++++ .../lyng/io/db/sqlite/PlatformNative.kt | 70 ++++++++----- 4 files changed, 294 insertions(+), 39 deletions(-) diff --git a/lyngio/src/jvmMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformJvm.kt b/lyngio/src/jvmMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformJvm.kt index 510269e..0933042 100644 --- a/lyngio/src/jvmMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformJvm.kt +++ b/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 = block(tx) - connection.commit() - result + val result = try { + block(tx) } catch (e: Throwable) { - rollbackQuietly(connection) - throw e + throw finishFailedTransaction(scope, core, 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 result = block(JdbcSqliteTransactionBackend(core, connection)) - connection.releaseSavepoint(savepoint) - result + val nested = JdbcSqliteTransactionBackend(core, connection) + val result = try { + block(nested) } catch (e: Throwable) { - rollbackQuietly(connection, savepoint) - throw e + throw finishFailedTransaction(scope, core, 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) - } catch (_: SQLException) { + connection.rollback() + } 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() diff --git a/lyngio/src/jvmTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleTest.kt b/lyngio/src/jvmTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleTest.kt index e9c259e..3fe7902 100644 --- a/lyngio/src/jvmTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleTest.kt +++ b/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 { + db.invokeInstanceMethod( + scope, + "transaction", + ObjExternCallable.fromBridge { + val tx = requiredArg(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(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 { + db.invokeInstanceMethod( + scope, + "transaction", + ObjExternCallable.fromBridge { + val tx = requiredArg(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 { + db.invokeInstanceMethod( + scope, + "transaction", + ObjExternCallable.fromBridge { + val tx = requiredArg(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() diff --git a/lyngio/src/linuxTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleNativeTest.kt b/lyngio/src/linuxTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleNativeTest.kt index 80bdda7..d44e961 100644 --- a/lyngio/src/linuxTest/kotlin/net/sergeych/lyng/io/db/sqlite/LyngSqliteModuleNativeTest.kt +++ b/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 { + db.invokeInstanceMethod( + scope, + "transaction", + ObjExternCallable.fromBridge { + val tx = requiredArg(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(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 { + db.invokeInstanceMethod( + scope, + "transaction", + ObjExternCallable.fromBridge { + val tx = requiredArg(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 { + db.invokeInstanceMethod( + scope, + "transaction", + ObjExternCallable.fromBridge { + val tx = requiredArg(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() diff --git a/lyngio/src/nativeMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformNative.kt b/lyngio/src/nativeMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformNative.kt index 51f1996..1316c77 100644 --- a/lyngio/src/nativeMain/kotlin/net/sergeych/lyng/io/db/sqlite/PlatformNative.kt +++ b/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 = block(tx) - handle.execUnit(scope, core, "commit") - result + val result = try { + block(tx) } catch (e: Throwable) { - handle.execUnitQuietly("rollback") - throw e + throw finishFailedTransaction(scope, core, 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 transaction(scope: ScopeFacade, block: suspend (SqliteTransactionBackend) -> T): T { val savepoint = "lyng_sp_${savepoints.next()}" handle.execUnit(scope, core, "savepoint $savepoint") - return try { - val result = block(NativeSqliteTransactionBackend(core, handle, savepoints)) - handle.execUnit(scope, core, "release savepoint $savepoint") - result + val nested = NativeSqliteTransactionBackend(core, handle, savepoints) + val result = try { + block(nested) } catch (e: Throwable) { - handle.execUnitQuietly("rollback to savepoint $savepoint") - handle.execUnitQuietly("release savepoint $savepoint") - throw e + throw finishFailedTransaction(scope, core, e) { + handle.execUnit(scope, core, "rollback to savepoint $savepoint") + 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) +}