You have muted a bug of the function above IMHO.

if (!$this->getIsActive()) {
    throw new Exception('Failed to commit transaction: transaction was inactive.');
}

Right, I was too hasty with accepting this. Is there a significant difference now between $this->getIsActive() and $this->db->pdo->inTransaction()?

@githubjeka As mentioned in #18406, I think it’s worth changing this behavior for Yii 3, but the point here is to avoid breaking backwards compatibility.

@bizley getIsActive() is only concerned with the internal record of the transaction, but is unaware of whether the transaction is actually still active, which won’t be the case if you’re running MySQL and the transaction had been implicitly committed due to a schema change.

I'm wondering if this check could be placed in $this->getIsActive().

Placing the check in $this->getIsActive() is an interesting idea. May work well. @brandonkelly what do you think?

Placing it there might be a good idea for Yii 3, but would be a BC break in the meantime, since commit() and rollBack() will each throw an exception if that returns false. So all it would change (versus doing nothing) is that commit() and rollBack() would start throwing yii\db\Exceptions rather than PDOExceptions.

rollBack() just returns nothing, only commit() would throw exception. And while I'm going through the code for me it looks like getIsActive() was never a good indicator whether the transaction is active (just checking a level and whether the PDO instance is set), it should always use PDO::inTransaction() as well). IMO the current code is bad and what @brandonkelly proposes should be placed in getIsActive() with upgrade info stating the users depending on previous code must update their apps, because simply ignoring autocommit was a bug. If the new check goes to getIsActive() it fixes also setIsolationLevel() because transaction must be active there as well.

I completely agree that this PR isn’t great, but any other change would be significantly more involved, and would introduce a breaking change.

This is the situation:

• yii\db\Transaction has always (ignorantly) assumed that it is the authority on whether a transaction is currently active, and at what “level”.
• That has never been accurate with MySQL, due to implicit commits that Yii was unaware of
• A change to that behavior is going to lead to lots of code breakage

I’m simply saying that for now, ignorance is bliss, and Yii should just look the other way when it turns out the transaction isn’t actually active even though its record states that it is.

Well, all you said is true. @samdark make a call please.

I think, current solution for Yii 2, getIsActive() for Yii 3 DB.