ORM and DBAL: Erroneous PDOException "There is no active transaction" thrown during commit #5169

Open
opened 2026-01-22 15:00:27 +01:00 by admin · 2 comments
Owner

Originally created by @nagmat84 on GitHub (Jun 29, 2016).

Overview

Assume that we rely on the implicit transaction handling of the Doctrine ORM EntityManager. Assume the following simple code

$em = $this->getDoctrine()->getManager();
$rep = $em->getRepository('MyBundle:MyEntity');
$entity = $rep->find( 42 );
$em->remove( $entity );
$em->flush();

and further assume that entity 42 cannot be removed due to violated foreign key constraints.

Expected result

The expected result is that $em->flush() throws some sort of exception that relates to the foreign key constraint, e.g. \Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException.

Actual result

The actual result is a native PHP PDOException with error message "There is no active transaction". The latter is an astonishing exception, because the aforementioned code does not deal with transactions directly.

Investigations and Proposed Solutions

Below I will explain where the bug stems from. The story is a little bit longer, because it involves 1 bug inside of the ORM component and 3 bugs inside of the DBAL component.

Reason 1: ORM component

Snippet from class \Doctrine\ORM\UnitOfWork

331: public function commit($entity = null)
332: {
...
373:     $conn->beginTransaction();
374:
375:     try {
...
403:         // Entity deletions come last and need to be in reverse commit order
404:         if ($this->entityDeletions) {
405:             for ($count = count($commitOrder), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) {
406:                 $this->executeDeletions($commitOrder[$i]);
407:             }
408:         }
409:
410:         $conn->commit();
411:     } catch (Exception $e) {
412:         $this->em->close();
413:         $conn->rollback();
414:
415:         $this->afterTransactionRolledBack();
416:
417:         throw $e;
418:     }
...
440: }

At line 410 while calling $conn->commit() the following exception is thrown:

$e = {PDOException} [9]
  message = "SQLSTATE[23503]: Foreign key violation: 7 ERROR:  update or delete on table "..." violates foreign key constraint "..." on table "..." DETAIL:  Key (id)=(42) is still referenced from table "..."."
  *Exception*string = ""
  code = "23503"
  file = "/srv/www/matthias.nagel/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php"
  line = 1216
  *Exception*trace = {array} [9]
  *Exception*previous = null
  errorInfo = {array} [3]
    0 = "23503"
    1 = "7"
    2 = "ERROR:  update or delete on table "..." violates foreign key constraint "..." on table "..." DETAIL:  Key (id)=(42) is still referenced from table "..."."
  xdebug_message = "..."

This is correct, because the foreign key constraint is evaluated during the commitment. However, the type of the exception should probably be a \Doctrine\DBAL\Exception\ForeignKeyException. We come to this point later, because this must be fixed elsewhere.

The problem here is that at line 413 inside of the catch-block $conn->rollback() tries to rollback a transaction that has already been terminated by $conn->commit(). It seems that this code does not take into account that $conn->commit() can fail with an exception after the transaction has already been terminated. In this case the transaction is automatically rolled back at server side and must not be rolled back a second time.

At the moment at line line 413 $conn->rollback() throws an additional exception and the original exception is lost, because line 417 will never be executed:

$e = {PDOException} [9]
  message ="There is no active transaction"
  *Exception*string = ""
  code = "0"
  file = "/srv/www/matthias.nagel/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php"
  line = 1278
  *Exception*trace = {array} [9]
  *Exception*previous = null
  errorInfo = null
  xdebug_message = "... left out ..."

Proposed solution (part 1 of 4): Check inside of the exeption handler if transaction is still active and only roll back in this case. Replace line 413 by: if( $conn->isTransactionActive() ) $conn->rollback()

However, even at the moment the exception thrown by rollback should be \Doctrine\DBAL\ConnectionException::noActiveTransaction() and not a plain PDOException. This dicrepancy is due to another bug. We come to this point later, because this must be fixed elsewhere.

Reason 2: DBAL component

The both remaining bugs that have already been mentioned are located at class \Doctrine\DBAL\Connection

1164: public function beginTransaction()
1165: {
...
1168:     ++$this->_transactionNestingLevel;
...
1172:     if ($this->_transactionNestingLevel == 1) {
...
1176:         $this->_conn->beginTransaction();
...
1180:     } elseif ($this->_nestTransactionsWithSavepoints) {
...
1188:     }
1189: }
...
1199: public function commit()
1200: {
1201:     if ($this->_transactionNestingLevel == 0) {
1202:         throw ConnectionException::noActiveTransaction();
1203:     }
...
1212:     if ($this->_transactionNestingLevel == 1) {
...
1216:         $this->_conn->commit();
...
1220:     } elseif ($this->_nestTransactionsWithSavepoints) {
...
1228:     }
1229:
1230:     --$this->_transactionNestingLevel;
...
1235: }
...
1263: public function rollBack()
1264: {
1265:     if ($this->_transactionNestingLevel == 0) {
1266:         throw ConnectionException::noActiveTransaction();
1267:     }
...
1273:     if ($this->_transactionNestingLevel == 1) {
...
1277:         $this->_transactionNestingLevel = 0;
1278:         $this->_conn->rollback();
...
1287:     } elseif ($this->_nestTransactionsWithSavepoints) {
...
1296:     } else {
...
1299:     }
1300: }

At line 1216 inside the commit-method the method $this->_conn->commit() of the underlying native PHP PDO object is called. Here the original PDOException due to the violated foreign key constraint is thrown. However, the code does not take into account that $this->_conn->commit() might fail with an exception and still terminates the transaction nonetheless. The _transactionNestingLevel should be decremented to 0, because the transaction has been terminated.

Proposed solution (part 2 of 4): Nest $this->_conn->commit() into a try-block, catch the PDOException , inside the catch-block check if the transaction has been terminated and decrement _transactionNestingLevel if necessary.

Proposed solution (part 3 of 4): Moreover, re-throw the PDOException but wrap or convert it into an appropriate child object of \Doctrine\DBAL\DBALException (in this particular case: \Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException).

At the moment the condition at line 1265 inside the rollback-method does not catch the aforementioned case. If the proposed solution is applied the _transactionNestingLevel would be zero and ConnectionException::noActiveTransaction() would be thrown. At the moment _transactionNestingLevel erroneously equals 1 and $this->_conn->rollback() throws a PDOException because there is no active transaction. This problem would be fixed with proposed solution part 2.

However, there are still one more bug in this code. (I am sorry.) In method beginTransaction() at line 1168 the _transactionNestingLevel is increased and the transaction is started at line 1176. The code does not take into account that $this->_conn->beginTransaction() might fail with an exception without starting a transaction. In this case _transactionNestingLevel erroneously equals 1 but should be 0.

Proposed solution (part 4 of 4): Nest $this->_conn->beginTransaction() into a try-block and decrement _transactionNestingLevel again, if the transaction could not be started.

Originally created by @nagmat84 on GitHub (Jun 29, 2016). # Overview Assume that we rely on the implicit transaction handling of the Doctrine ORM EntityManager. Assume the following simple code ``` $em = $this->getDoctrine()->getManager(); $rep = $em->getRepository('MyBundle:MyEntity'); $entity = $rep->find( 42 ); $em->remove( $entity ); $em->flush(); ``` and further assume that entity 42 cannot be removed due to violated foreign key constraints. ### Expected result The _expected result_ is that `$em->flush()` throws some sort of exception that relates to the foreign key constraint, e.g. `\Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException`. ### Actual result The _actual result_ is a native PHP `PDOException` with error message "There is no active transaction". The latter is an astonishing exception, because the aforementioned code does not deal with transactions directly. # Investigations and Proposed Solutions Below I will explain where the bug stems from. The story is a little bit longer, because it involves 1 bug inside of the ORM component and 3 bugs inside of the DBAL component. ## Reason 1: ORM component Snippet from class `\Doctrine\ORM\UnitOfWork` ``` 331: public function commit($entity = null) 332: { ... 373: $conn->beginTransaction(); 374: 375: try { ... 403: // Entity deletions come last and need to be in reverse commit order 404: if ($this->entityDeletions) { 405: for ($count = count($commitOrder), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) { 406: $this->executeDeletions($commitOrder[$i]); 407: } 408: } 409: 410: $conn->commit(); 411: } catch (Exception $e) { 412: $this->em->close(); 413: $conn->rollback(); 414: 415: $this->afterTransactionRolledBack(); 416: 417: throw $e; 418: } ... 440: } ``` At line 410 while calling `$conn->commit()` the following exception is thrown: ``` $e = {PDOException} [9] message = "SQLSTATE[23503]: Foreign key violation: 7 ERROR: update or delete on table "..." violates foreign key constraint "..." on table "..." DETAIL: Key (id)=(42) is still referenced from table "..."." *Exception*string = "" code = "23503" file = "/srv/www/matthias.nagel/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php" line = 1216 *Exception*trace = {array} [9] *Exception*previous = null errorInfo = {array} [3] 0 = "23503" 1 = "7" 2 = "ERROR: update or delete on table "..." violates foreign key constraint "..." on table "..." DETAIL: Key (id)=(42) is still referenced from table "..."." xdebug_message = "..." ``` This is correct, because the foreign key constraint is evaluated during the commitment. However, the type of the exception should probably be a `\Doctrine\DBAL\Exception\ForeignKeyException`. We come to this point later, because this must be fixed elsewhere. The problem here is that at line 413 inside of the catch-block `$conn->rollback()` tries to rollback a transaction that has already been terminated by `$conn->commit()`. It seems that this code does not take into account that `$conn->commit()` can fail with an exception _after_ the transaction has already been terminated. In this case the transaction is automatically rolled back at server side and _must not_ be rolled back a second time. At the moment at line line 413 `$conn->rollback()` throws an additional exception and the original exception is lost, because line 417 will never be executed: ``` $e = {PDOException} [9] message ="There is no active transaction" *Exception*string = "" code = "0" file = "/srv/www/matthias.nagel/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php" line = 1278 *Exception*trace = {array} [9] *Exception*previous = null errorInfo = null xdebug_message = "... left out ..." ``` **Proposed solution (part 1 of 4):** Check inside of the exeption handler if transaction is still active and only roll back in this case. Replace line 413 by: `if( $conn->isTransactionActive() ) $conn->rollback()` However, even at the moment the exception thrown by rollback should be `\Doctrine\DBAL\ConnectionException::noActiveTransaction()` and not a plain PDOException. This dicrepancy is due to another bug. We come to this point later, because this must be fixed elsewhere. ## Reason 2: DBAL component The both remaining bugs that have already been mentioned are located at class `\Doctrine\DBAL\Connection` ``` 1164: public function beginTransaction() 1165: { ... 1168: ++$this->_transactionNestingLevel; ... 1172: if ($this->_transactionNestingLevel == 1) { ... 1176: $this->_conn->beginTransaction(); ... 1180: } elseif ($this->_nestTransactionsWithSavepoints) { ... 1188: } 1189: } ... 1199: public function commit() 1200: { 1201: if ($this->_transactionNestingLevel == 0) { 1202: throw ConnectionException::noActiveTransaction(); 1203: } ... 1212: if ($this->_transactionNestingLevel == 1) { ... 1216: $this->_conn->commit(); ... 1220: } elseif ($this->_nestTransactionsWithSavepoints) { ... 1228: } 1229: 1230: --$this->_transactionNestingLevel; ... 1235: } ... 1263: public function rollBack() 1264: { 1265: if ($this->_transactionNestingLevel == 0) { 1266: throw ConnectionException::noActiveTransaction(); 1267: } ... 1273: if ($this->_transactionNestingLevel == 1) { ... 1277: $this->_transactionNestingLevel = 0; 1278: $this->_conn->rollback(); ... 1287: } elseif ($this->_nestTransactionsWithSavepoints) { ... 1296: } else { ... 1299: } 1300: } ``` At line 1216 inside the commit-method the method `$this->_conn->commit()` of the underlying native PHP PDO object is called. Here the original `PDOException` due to the violated foreign key constraint is thrown. However, the code does not take into account that `$this->_conn->commit()` might fail with an exception and still terminates the transaction nonetheless. The `_transactionNestingLevel` should be decremented to 0, because the transaction has been terminated. **Proposed solution (part 2 of 4):** Nest `$this->_conn->commit()` into a `try`-block, catch the `PDOException` , inside the `catch`-block check if the transaction has been terminated and decrement `_transactionNestingLevel` if necessary. **Proposed solution (part 3 of 4):** Moreover, re-throw the `PDOException` but wrap or convert it into an appropriate child object of `\Doctrine\DBAL\DBALException` (in this particular case: `\Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException`). At the moment the condition at line 1265 inside the rollback-method does not catch the aforementioned case. If the proposed solution is applied the `_transactionNestingLevel` would be zero and `ConnectionException::noActiveTransaction()` would be thrown. At the moment `_transactionNestingLevel` erroneously equals 1 and `$this->_conn->rollback()` throws a `PDOException` because there is no active transaction. This problem would be fixed with proposed solution part 2. However, there are still one more bug in this code. (I am sorry.) In method `beginTransaction()` at line 1168 the `_transactionNestingLevel` is increased and the transaction is started at line 1176. The code does not take into account that `$this->_conn->beginTransaction()` might fail with an exception without starting a transaction. In this case `_transactionNestingLevel` erroneously equals 1 but should be 0. **Proposed solution (part 4 of 4):** Nest `$this->_conn->beginTransaction()` into a `try`-block and decrement `_transactionNestingLevel` again, if the transaction could not be started.
Author
Owner

@deeky666 commented on GitHub (Jun 30, 2016):

@nagmat84 first of all thanks for the very detailed issue description and investigation. There seem to be some valid points here, although I want to point out two things:

  1. It's not all about PDO so your expectations about PDOExceptions are only true for PDO drivers and other might most certainly react differently.
  2. I think the root problem of the Doctrine code not catching exceptions from commit() and beginTransaction() calls is that the related DBAL connection interface does not define exceptions to be thrown but return true or false instead. This might have been a design mistake or by intention I don't know. The question here is then whether we can redefine the interface description to throw exceptions (which it currently partially does anyways) or if driver implementations have to be adjusted. I personally would prefer the first as error information might get lost otherwise.

That pointed out, I might take a look at the DBAL side issues as soon as I have time.

Thanks for reporting.

@deeky666 commented on GitHub (Jun 30, 2016): @nagmat84 first of all thanks for the very detailed issue description and investigation. There seem to be some valid points here, although I want to point out two things: 1. It's not all about PDO so your expectations about PDOExceptions are only true for PDO drivers and other might most certainly react differently. 2. I think the root problem of the Doctrine code not catching exceptions from `commit()` and `beginTransaction()` calls is that the related DBAL connection interface does not define exceptions to be thrown but return `true` or `false` instead. This might have been a design mistake or by intention I don't know. The question here is then whether we can redefine the interface description to throw exceptions (which it currently partially does anyways) or if driver implementations have to be adjusted. I personally would prefer the first as error information might get lost otherwise. That pointed out, I might take a look at the DBAL side issues as soon as I have time. Thanks for reporting.
Author
Owner

@nagmat84 commented on GitHub (Jun 30, 2016):

@deeky666 thank you for your reply and your offer to look at it. Regarding your points

  1. I see your point. I only found Doctrine/DBAL/Driver/PDOConnection.php, so this was my mistake. I have the feeling that PHP PDO behaved differently in the past, too. PHP PDO methods commit(), beginTransaction() and rollback() can either throw exceptions or return true or false. That depends on what the attribute PDO::ATTR_ERRMODE is set to. I have the strange feeling that I did not encounter this problem in the past and maybe the PHP default settings have changed. But this is just a guess.

  2. You are right in this point. If a failing commit() does not throw an exception but just returns false the current code would at least not throw an exception and behave more or less correctly. However, even if you stick to return booleans the following code \Doctrine\ORM\UnitOfWork is still not perfect:

    331: public function commit($entity = null)
    332: {
    ...
    373:     $conn->beginTransaction();
    374:
    375:     try {
    ...          // do some SQL stuff
    410:         $conn->commit();
    411:     } catch (Exception $e) {
    412:         $this->em->close();
    413:         $conn->rollback();
    414:
    415:         $this->afterTransactionRolledBack();
    416:
    417:         throw $e;
    418:     }
    ...
    440: }
    

    A failing commitment without an exception would at least not trigger a superfluous rollback and it would correctly decrement the _transactionNestingLevel but the error would never be reported to the user code. The return code of commit() is not checked. If one sticks to return booleans they should be checked at least.

I would like to raise another point. Personally, from looking at the code I have the feeling that the whole code is written without taking into account that SQL constraints can be deferred within a transaction until the end of the transaction. I have the feeling that the whole code expects each query to throw an exception immediately, if the SQL statement cannot be executed. Hence, if the DBMS, the SQL table or the SQL constraint is defined to be "immediately", we would have the following result:

> START TRANSACTION;
Success: Transaction started
> DELETE FROM table where id = 42;
Error 4711: Foreign key constraint violated
> ROLLBACK
Success: Transaction cancelled

The above works just fine with DBAL. But if the DBMS, the SQL table or constraint is "deferred", we would have the following result:

> START TRANSACTION;
Success: Transaction started
> DELETE FROM table where id = 42;
Success: 1 row affected
> COMMIT
Error: Deferred foreign key constraint violated, transaction cancelled
> ROLLBACK
Error: There is no active transaction

Probably, I wrong you guys, but I have the impression that this aspect was overseen.

@nagmat84 commented on GitHub (Jun 30, 2016): @deeky666 thank you for your reply and your offer to look at it. Regarding your points 1. I see your point. I only found `Doctrine/DBAL/Driver/PDOConnection.php`, so this was my mistake. I have the feeling that PHP PDO behaved differently in the past, too. PHP PDO methods `commit()`, `beginTransaction()` and `rollback()` can either throw exceptions or return `true` or `false`. That depends on what the attribute `PDO::ATTR_ERRMODE` is set to. I have the strange feeling that I did not encounter this problem in the past and maybe the PHP default settings have changed. But this is just a guess. 2. You are right in this point. If a failing `commit()` does not throw an exception but just returns `false` the current code would at least not throw an exception and behave more or less correctly. However, even if you stick to return booleans the following code `\Doctrine\ORM\UnitOfWork` is still not perfect: ``` 331: public function commit($entity = null) 332: { ... 373: $conn->beginTransaction(); 374: 375: try { ... // do some SQL stuff 410: $conn->commit(); 411: } catch (Exception $e) { 412: $this->em->close(); 413: $conn->rollback(); 414: 415: $this->afterTransactionRolledBack(); 416: 417: throw $e; 418: } ... 440: } ``` A failing commitment without an exception would at least not trigger a superfluous rollback and it would correctly decrement the `_transactionNestingLevel` but the error would never be reported to the user code. The return code of `commit()` is not checked. If one sticks to return booleans they should be checked at least. I would like to raise another point. Personally, from looking at the code I have the feeling that the whole code is written without taking into account that SQL constraints can be deferred within a transaction until the end of the transaction. I have the feeling that the whole code expects each query to throw an exception immediately, if the SQL statement cannot be executed. Hence, if the DBMS, the SQL table or the SQL constraint is defined to be "_immediately_", we would have the following result: ``` > START TRANSACTION; Success: Transaction started > DELETE FROM table where id = 42; Error 4711: Foreign key constraint violated > ROLLBACK Success: Transaction cancelled ``` The above works just fine with DBAL. But if the DBMS, the SQL table or constraint is "_deferred_", we would have the following result: ``` > START TRANSACTION; Success: Transaction started > DELETE FROM table where id = 42; Success: 1 row affected > COMMIT Error: Deferred foreign key constraint violated, transaction cancelled > ROLLBACK Error: There is no active transaction ``` Probably, I wrong you guys, but I have the impression that this aspect was overseen.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#5169