Attempting to rollback a connection that fails to get created

[mdnorman]

At this line, the beginTransaction call could fail, throwing an error that gets caught on line 570. The issue we are running into is that rollbackTransaction then gets called on a transaction that's not created yet. In the driver we're using, rolling back a transaction that's not created yet is considered an error.

I could go either way on this one, so is it a bug in the driver code that rollbackTransaction shouldn't throw an error if it's not in a transaction yet, or is it a bug to call rollbackTransaction when the transaction hasn't been created yet.

If the former, I'll go open an issue against the driver to put a fix in. If it's the latter, then I'm hoping it can get fixed here.

[koskimas]

Nice catch (pun intended)! This should be fixed in Kysely.

On the other hand, beginTransaction can do multiple things. For example mysql creates a transaction and sets the isolation level using a separate query. If the second query fails, we still need to roll back the transaction.

But leaving this up to the driver seems risky too.

[igalklebanov]

Hey 👋

On the other hand, beginTransaction can do multiple things.

I verified all of our present and future core dialects don't perform a secondary query AFTER beginning the transaction in beginTransaction.

I also checked organizational and community dialects. None do it.

For example mysql creates a transaction and sets the isolation level using a separate query. If the second query fails, we still need to roll back the transaction.

Actually the other way around. It first configures the connection for the upcoming transaction, and only after, begins the transaction. If query 1 fails, no transaction is started yet.

https://github.com/kysely-org/kysely/blob/3f1db74463fa1ac37f551fbae1a4fac83ccef816/src/dialect/mysql/mysql-driver.ts#L89-L93

But leaving this up to the driver seems risky too.

100%. The naive solution is flipping a flag in TransactionBuilder's execute right after this.#props.driver.beginTransaction(...) succeeds, which then can help decide whether to call this.#props.driver.rollbackTransaction(...) in the catch block.

If drivers in the wild do multiple queries in beginTransaction, and beginning the transaction is not the last query, we could, add a hasBegunTransaction?(connection) method to Driver that will give them the power to tell TransactionBuilder if it begun or not.