Skip to content

Commit

Permalink
Run risky code in finally block
Browse files Browse the repository at this point in the history
catch blocks are not supposed to fail. If you want to do something
despite an exception happening, you should do it in a finally block.

Closes #7545
  • Loading branch information
greg0ire committed Oct 9, 2024
1 parent bc37f75 commit a62c5df
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 19 deletions.
29 changes: 18 additions & 11 deletions src/EntityManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
use Doctrine\Persistence\Mapping\MappingException;
use Doctrine\Persistence\ObjectRepository;
use InvalidArgumentException;
use Throwable;

use function array_keys;
use function class_exists;
Expand Down Expand Up @@ -246,18 +245,22 @@ public function transactional($func)

$this->conn->beginTransaction();

$successful = false;

try {
$return = $func($this);

$this->flush();
$this->conn->commit();

return $return ?: true;
} catch (Throwable $e) {
$this->close();
$this->conn->rollBack();
$successful = true;

throw $e;
return $return ?: true;
} finally {
if (! $successful) {
$this->close();
$this->conn->rollBack();
}
}
}

Expand All @@ -268,18 +271,22 @@ public function wrapInTransaction(callable $func)
{
$this->conn->beginTransaction();

$successful = false;

try {
$return = $func($this);

$this->flush();
$this->conn->commit();

return $return;
} catch (Throwable $e) {
$this->close();
$this->conn->rollBack();
$successful = true;

throw $e;
return $return;
} finally {
if (! $successful) {
$this->close();
$this->conn->rollBack();
}
}
}

Expand Down
19 changes: 11 additions & 8 deletions src/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
use Exception;
use InvalidArgumentException;
use RuntimeException;
use Throwable;
use UnexpectedValueException;

use function array_chunk;
Expand Down Expand Up @@ -427,6 +426,8 @@ public function commit($entity = null)
$conn = $this->em->getConnection();
$conn->beginTransaction();

$successful = false;

try {
// Collection deletions (deletions of complete collections)
foreach ($this->collectionDeletions as $collectionToDelete) {
Expand Down Expand Up @@ -478,16 +479,18 @@ public function commit($entity = null)

throw new OptimisticLockException('Commit failed', $object);
}
} catch (Throwable $e) {
$this->em->close();

if ($conn->isTransactionActive()) {
$conn->rollBack();
}
$successful = true;
} finally {
if (! $successful) {
$this->em->close();

$this->afterTransactionRolledBack();
if ($conn->isTransactionActive()) {
$conn->rollBack();
}

throw $e;
$this->afterTransactionRolledBack();
}
}

$this->afterTransactionComplete();
Expand Down

0 comments on commit a62c5df

Please sign in to comment.