Fix incorrect handling of transactions using deferred constraints

- Let root Errors bubble up
- Catch concrete Exception in TransactionTest as we can do it now
- Expose underlying errors on Oracle platform

Co-authored-by: Jakub Chábek <jakub.chabek@cdn77.com>

src/Connection.php

@@ -1202,14 +1202,15 @@ public function transactional(Closure $func)
         $this->beginTransaction();
         try {
             $res = $func($this);
-            $this->commit();
-
-            return $res;
         } catch (Throwable $e) {
             $this->rollBack();
 
             throw $e;
         }
+
+        $this->commit();
+
+        return $res;
     }
 
     /**
@@ -1320,7 +1321,13 @@ public function commit()
                 $logger->startQuery('"COMMIT"');
             }
 
-            $result = $connection->commit();
+            try {
+                $result = $connection->commit();
+            } catch (Driver\Exception $e) {
+                $this->updateTransactionStateAfterCommit();
+
+                throw $this->convertExceptionDuringQuery($e, 'COMMIT');
+            }
 
             if ($logger !== null) {
                 $logger->stopQuery();
@@ -1336,17 +1343,22 @@ public function commit()
             }
         }
 
+        $this->updateTransactionStateAfterCommit();
+
+        return $result;
+    }
+
+    private function updateTransactionStateAfterCommit(): void
+    {
         --$this->transactionNestingLevel;
 
         $this->getEventManager()->dispatchEvent(Events::onTransactionCommit, new TransactionCommitEventArgs($this));
 
         if ($this->autoCommit !== false || $this->transactionNestingLevel !== 0) {
-            return $result;
+            return;
         }
 
         $this->beginTransaction();
-
-        return $result;
     }
 
     /**

src/Driver/OCI8/Connection.php

@@ -156,7 +156,7 @@ public function beginTransaction()
      */
     public function commit()
     {
-        if (! oci_commit($this->dbh)) {
+        if (! @oci_commit($this->dbh)) {
             throw Error::new($this->dbh);
         }
 

src/Driver/OCI8/Exception/Error.php

@@ -7,7 +7,9 @@
 use Doctrine\DBAL\Driver\AbstractException;
 
 use function assert;
+use function explode;
 use function oci_error;
+use function str_replace;
 
 /**
  * @internal
@@ -16,6 +18,8 @@
  */
 final class Error extends AbstractException
 {
+    private const CODE_TRANSACTION_ROLLED_BACK = 2091;
+
     /**
      * @param resource $resource
      */
@@ -24,6 +28,18 @@ public static function new($resource): self
         $error = oci_error($resource);
         assert($error !== false);
 
-        return new self($error['message'], null, $error['code']);
+        $code    = $error['code'];
+        $message = $error['message'];
+        if ($code === self::CODE_TRANSACTION_ROLLED_BACK) {
+            // There's no way this can be unit-tested as it's impossible to mock $resource
+            //ORA-02091: transaction rolled back
+            //ORA-00001: unique constraint (DOCTRINE.GH3423_UNIQUE) violated
+            [$firstMessage, $secondMessage] = explode("\n", $message, 2);
+
+            [$code, $message] = explode(': ', $secondMessage, 2);
+            $code             = (int) str_replace('ORA-', '', $code);
+        }
+
+        return new self($message, null, $code);
     }
 }

src/Driver/PDO/Connection.php

@@ -140,7 +140,11 @@ public function beginTransaction()
      */
     public function commit()
     {
-        return $this->connection->commit();
+        try {
+            return $this->connection->commit();
+        } catch (PDOException $exception) {
+            throw Exception::new($exception);
+        }
     }
 
     /**

src/Driver/PDO/Exception.php

@@ -7,22 +7,38 @@
 use Doctrine\DBAL\Driver\AbstractException;
 use PDOException;
 
+use function explode;
+use function str_replace;
+
 /**
  * @internal
  *
  * @psalm-immutable
  */
 final class Exception extends AbstractException
 {
+    private const CODE_TRANSACTION_ROLLED_BACK = 2091;
+
     public static function new(PDOException $exception): self
     {
+        $message = $exception->getMessage();
+
         if ($exception->errorInfo !== null) {
             [$sqlState, $code] = $exception->errorInfo;
         } else {
             $code     = $exception->getCode();
             $sqlState = null;
         }
 
-        return new self($exception->getMessage(), $sqlState, $code, $exception);
+        if ($code === self::CODE_TRANSACTION_ROLLED_BACK) {
+            //ORA-02091: transaction rolled back
+            //ORA-00001: unique constraint (DOCTRINE.GH3423_UNIQUE) violated
+            [$firstMessage, $secondMessage] = explode("\n", $message, 2);
+
+            [$code, $message] = explode(': ', $secondMessage, 2);
+            $code             = (int) str_replace('ORA-', '', $code);
+        }
+
+        return new self($message, $sqlState, $code, $exception);
     }
 }

tests/Driver/PDO/ExceptionTest.php

@@ -61,4 +61,25 @@ public function testOriginalExceptionIsInChain(): void
     {
         self::assertSame($this->wrappedException, $this->exception->getPrevious());
     }
+
+    public function testExposesUnderlyingErrorOnOracle(): void
+    {
+        $pdoException = new PDOException(<<<TEXT
+OCITransCommit: ORA-02091: transaction rolled back
+ORA-00001: unique constraint (DOCTRINE.C1_UNIQUE) violated
+ (/private/tmp/php-20211003-35441-1sggrmq/php-8.0.11/ext/pdo_oci/oci_driver.c:410)
+TEXT);
+
+        $pdoException->errorInfo = [self::SQLSTATE, 2091,
+
+        ];
+
+        $exception = Exception::new($pdoException);
+
+        self::assertSame(1, $exception->getCode());
+        self::assertStringContainsString(
+            'unique constraint (DOCTRINE.C1_UNIQUE) violated',
+            $exception->getMessage()
+        );
+    }
 }