From 874a5e3547d08c6be62aa8bae2f1afc380af983d Mon Sep 17 00:00:00 2001 From: Benjamin Morel Date: Thu, 21 Apr 2016 23:33:34 +0200 Subject: [PATCH 1/4] Catch Throwable in PHP 7 --- lib/Doctrine/ORM/EntityManager.php | 8 ++++++-- .../ORM/Query/Exec/MultiTableDeleteExecutor.php | 6 +++++- .../ORM/Query/Exec/MultiTableUpdateExecutor.php | 6 +++++- .../Command/EnsureProductionSettingsCommand.php | 6 +++++- lib/Doctrine/ORM/Tools/SchemaTool.php | 8 ++++++-- lib/Doctrine/ORM/Tools/ToolsException.php | 4 ++-- lib/Doctrine/ORM/UnitOfWork.php | 10 +++++++++- 7 files changed, 38 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index d4b032bfc..99a9b15dc 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -19,7 +19,6 @@ namespace Doctrine\ORM; -use Exception; use Doctrine\Common\EventManager; use Doctrine\DBAL\Connection; use Doctrine\DBAL\DriverManager; @@ -237,7 +236,12 @@ use Doctrine\Common\Util\ClassUtils; $this->conn->commit(); return $return ?: true; - } catch (Exception $e) { + } catch (\Throwable $e) { + $this->close(); + $this->conn->rollBack(); + + throw $e; + } catch (\Exception $e) { // PHP 5 $this->close(); $this->conn->rollBack(); diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php index aaf94d7ff..1ecd7a7b2 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php @@ -129,11 +129,15 @@ class MultiTableDeleteExecutor extends AbstractSqlExecutor foreach ($this->_sqlStatements as $sql) { $conn->executeUpdate($sql); } - } catch (\Exception $exception) { + } catch (\Throwable $exception) { // FAILURE! Drop temporary table to avoid possible collisions $conn->executeUpdate($this->_dropTempTableSql); // Re-throw exception + throw $exception; + } catch (\Exception $exception) { // PHP 5 + $conn->executeUpdate($this->_dropTempTableSql); + throw $exception; } diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php index acad25a92..296b7d553 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php @@ -188,11 +188,15 @@ class MultiTableUpdateExecutor extends AbstractSqlExecutor $conn->executeUpdate($statement, $paramValues, $paramTypes); } - } catch (\Exception $exception) { + } catch (\Throwable $exception) { // FAILURE! Drop temporary table to avoid possible collisions $conn->executeUpdate($this->_dropTempTableSql); // Re-throw exception + throw $exception; + } catch (\Exception $exception) { // PHP 5 + $conn->executeUpdate($this->_dropTempTableSql); + throw $exception; } diff --git a/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php b/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php index 499565a01..e628b7049 100644 --- a/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php +++ b/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php @@ -72,7 +72,11 @@ EOT if ($input->getOption('complete') !== null) { $em->getConnection()->connect(); } - } catch (\Exception $e) { + } catch (\Throwable $e) { + $output->writeln('' . $e->getMessage() . ''); + + return 1; + } catch (\Exception $e) { // PHP 5 $output->writeln('' . $e->getMessage() . ''); return 1; diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 007ac85a8..e84a9c53d 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -92,7 +92,9 @@ class SchemaTool foreach ($createSchemaSql as $sql) { try { $conn->executeQuery($sql); - } catch (\Exception $e) { + } catch (\Throwable $e) { + throw ToolsException::schemaToolFailure($sql, $e); + } catch (\Exception $e) { // PHP 5 throw ToolsException::schemaToolFailure($sql, $e); } } @@ -742,7 +744,9 @@ class SchemaTool foreach ($dropSchemaSql as $sql) { try { $conn->executeQuery($sql); - } catch (\Exception $e) { + } catch (\Throwable $e) { + + } catch (\Exception $e) { // PHP 5 } } diff --git a/lib/Doctrine/ORM/Tools/ToolsException.php b/lib/Doctrine/ORM/Tools/ToolsException.php index 0a4616404..ed6331118 100644 --- a/lib/Doctrine/ORM/Tools/ToolsException.php +++ b/lib/Doctrine/ORM/Tools/ToolsException.php @@ -30,11 +30,11 @@ class ToolsException extends ORMException { /** * @param string $sql - * @param \Exception $e + * @param \Exception $e The original exception, or duck-typed Throwable in PHP 7. * * @return ToolsException */ - public static function schemaToolFailure($sql, \Exception $e) + public static function schemaToolFailure($sql, $e) { return new self("Schema-Tool failed with Error '" . $e->getMessage() . "' while executing DDL: " . $sql, "0", $e); } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 0edd756f0..c5598cd4b 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -44,6 +44,7 @@ use Doctrine\ORM\Persisters\Entity\SingleTablePersister; use Doctrine\ORM\Proxy\Proxy; use Doctrine\ORM\Utility\IdentifierFlattener; use Exception; +use Throwable; use InvalidArgumentException; use UnexpectedValueException; @@ -411,7 +412,14 @@ class UnitOfWork implements PropertyChangedListener } $conn->commit(); - } catch (Exception $e) { + } catch (\Throwable $e) { + $this->em->close(); + $conn->rollBack(); + + $this->afterTransactionRolledBack(); + + throw $e; + } catch (\Exception $e) { // PHP 5 $this->em->close(); $conn->rollBack(); From a1c93bfd485368594cdff88329a6dee872dfcadf Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 2 Sep 2017 13:44:12 +0200 Subject: [PATCH 2/4] #5796 replacing `Exception` catching with `Throwable` catching, removing PHP5 compliance code --- lib/Doctrine/ORM/EntityManager.php | 5 ----- lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php | 4 ---- lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php | 4 ---- .../Console/Command/EnsureProductionSettingsCommand.php | 4 ---- lib/Doctrine/ORM/Tools/SchemaTool.php | 4 +--- lib/Doctrine/ORM/UnitOfWork.php | 9 --------- 6 files changed, 1 insertion(+), 29 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 99a9b15dc..49d7a372b 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -240,11 +240,6 @@ use Doctrine\Common\Util\ClassUtils; $this->close(); $this->conn->rollBack(); - throw $e; - } catch (\Exception $e) { // PHP 5 - $this->close(); - $this->conn->rollBack(); - throw $e; } } diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php index 1ecd7a7b2..e7287b708 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php @@ -134,10 +134,6 @@ class MultiTableDeleteExecutor extends AbstractSqlExecutor $conn->executeUpdate($this->_dropTempTableSql); // Re-throw exception - throw $exception; - } catch (\Exception $exception) { // PHP 5 - $conn->executeUpdate($this->_dropTempTableSql); - throw $exception; } diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php index 296b7d553..5dd9b1514 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php @@ -193,10 +193,6 @@ class MultiTableUpdateExecutor extends AbstractSqlExecutor $conn->executeUpdate($this->_dropTempTableSql); // Re-throw exception - throw $exception; - } catch (\Exception $exception) { // PHP 5 - $conn->executeUpdate($this->_dropTempTableSql); - throw $exception; } diff --git a/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php b/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php index e628b7049..5876d2d1f 100644 --- a/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php +++ b/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php @@ -75,10 +75,6 @@ EOT } catch (\Throwable $e) { $output->writeln('' . $e->getMessage() . ''); - return 1; - } catch (\Exception $e) { // PHP 5 - $output->writeln('' . $e->getMessage() . ''); - return 1; } diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index e84a9c53d..a0d3af2b4 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -745,9 +745,7 @@ class SchemaTool try { $conn->executeQuery($sql); } catch (\Throwable $e) { - - } catch (\Exception $e) { // PHP 5 - + // ignored } } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index c5598cd4b..09bc313c1 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -43,8 +43,6 @@ use Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister; use Doctrine\ORM\Persisters\Entity\SingleTablePersister; use Doctrine\ORM\Proxy\Proxy; use Doctrine\ORM\Utility\IdentifierFlattener; -use Exception; -use Throwable; use InvalidArgumentException; use UnexpectedValueException; @@ -418,13 +416,6 @@ class UnitOfWork implements PropertyChangedListener $this->afterTransactionRolledBack(); - throw $e; - } catch (\Exception $e) { // PHP 5 - $this->em->close(); - $conn->rollBack(); - - $this->afterTransactionRolledBack(); - throw $e; } From 12043cd845f9cf84f5f995a3ef25b716db541d92 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 2 Sep 2017 13:47:58 +0200 Subject: [PATCH 3/4] #5796 minor CS fixes (imported symbols) and removing last PHP5 compliance bits --- lib/Doctrine/ORM/EntityManager.php | 3 ++- lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php | 3 ++- lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php | 3 ++- .../Console/Command/EnsureProductionSettingsCommand.php | 3 ++- lib/Doctrine/ORM/Tools/SchemaTool.php | 2 -- lib/Doctrine/ORM/Tools/ToolsException.php | 9 ++------- lib/Doctrine/ORM/UnitOfWork.php | 3 ++- 7 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 49d7a372b..839505345 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -27,6 +27,7 @@ use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\ORM\Proxy\ProxyFactory; use Doctrine\ORM\Query\FilterCollection; use Doctrine\Common\Util\ClassUtils; +use Throwable; /** * The EntityManager is the central access point to ORM functionality. @@ -236,7 +237,7 @@ use Doctrine\Common\Util\ClassUtils; $this->conn->commit(); return $return ?: true; - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->close(); $this->conn->rollBack(); diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php index e7287b708..2f3d5acf5 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableDeleteExecutor.php @@ -23,6 +23,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Types\Type; use Doctrine\ORM\Query\AST; use Doctrine\ORM\Utility\PersisterHelper; +use Throwable; /** * Executes the SQL statements for bulk DQL DELETE statements on classes in @@ -129,7 +130,7 @@ class MultiTableDeleteExecutor extends AbstractSqlExecutor foreach ($this->_sqlStatements as $sql) { $conn->executeUpdate($sql); } - } catch (\Throwable $exception) { + } catch (Throwable $exception) { // FAILURE! Drop temporary table to avoid possible collisions $conn->executeUpdate($this->_dropTempTableSql); diff --git a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php index 5dd9b1514..7f13ed5f6 100644 --- a/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php @@ -24,6 +24,7 @@ use Doctrine\DBAL\Types\Type; use Doctrine\ORM\Query\ParameterTypeInferer; use Doctrine\ORM\Query\AST; use Doctrine\ORM\Utility\PersisterHelper; +use Throwable; /** * Executes the SQL statements for bulk DQL UPDATE statements on classes in @@ -188,7 +189,7 @@ class MultiTableUpdateExecutor extends AbstractSqlExecutor $conn->executeUpdate($statement, $paramValues, $paramTypes); } - } catch (\Throwable $exception) { + } catch (Throwable $exception) { // FAILURE! Drop temporary table to avoid possible collisions $conn->executeUpdate($this->_dropTempTableSql); diff --git a/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php b/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php index 5876d2d1f..180254f9c 100644 --- a/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php +++ b/lib/Doctrine/ORM/Tools/Console/Command/EnsureProductionSettingsCommand.php @@ -23,6 +23,7 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Throwable; /** * Command to ensure that Doctrine is properly configured for a production environment. @@ -72,7 +73,7 @@ EOT if ($input->getOption('complete') !== null) { $em->getConnection()->connect(); } - } catch (\Throwable $e) { + } catch (Throwable $e) { $output->writeln('' . $e->getMessage() . ''); return 1; diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index a0d3af2b4..6609b5ea0 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -94,8 +94,6 @@ class SchemaTool $conn->executeQuery($sql); } catch (\Throwable $e) { throw ToolsException::schemaToolFailure($sql, $e); - } catch (\Exception $e) { // PHP 5 - throw ToolsException::schemaToolFailure($sql, $e); } } } diff --git a/lib/Doctrine/ORM/Tools/ToolsException.php b/lib/Doctrine/ORM/Tools/ToolsException.php index ed6331118..95b3af0ac 100644 --- a/lib/Doctrine/ORM/Tools/ToolsException.php +++ b/lib/Doctrine/ORM/Tools/ToolsException.php @@ -20,6 +20,7 @@ namespace Doctrine\ORM\Tools; use Doctrine\ORM\ORMException; +use Throwable; /** * Tools related Exceptions. @@ -28,13 +29,7 @@ use Doctrine\ORM\ORMException; */ class ToolsException extends ORMException { - /** - * @param string $sql - * @param \Exception $e The original exception, or duck-typed Throwable in PHP 7. - * - * @return ToolsException - */ - public static function schemaToolFailure($sql, $e) + public static function schemaToolFailure(string $sql, Throwable $e) : self { return new self("Schema-Tool failed with Error '" . $e->getMessage() . "' while executing DDL: " . $sql, "0", $e); } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 09bc313c1..05ad599a4 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -44,6 +44,7 @@ use Doctrine\ORM\Persisters\Entity\SingleTablePersister; use Doctrine\ORM\Proxy\Proxy; use Doctrine\ORM\Utility\IdentifierFlattener; use InvalidArgumentException; +use Throwable; use UnexpectedValueException; /** @@ -410,7 +411,7 @@ class UnitOfWork implements PropertyChangedListener } $conn->commit(); - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->em->close(); $conn->rollBack(); From c016e2d434fd0c6ec108e227b85a092d6f807dc0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 2 Sep 2017 13:55:07 +0200 Subject: [PATCH 4/4] Adding minimal test to verify `EntityManager` behavior against #5796 --- tests/Doctrine/Tests/ORM/EntityManagerTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/EntityManagerTest.php b/tests/Doctrine/Tests/ORM/EntityManagerTest.php index 7f36bb285..0191ba158 100644 --- a/tests/Doctrine/Tests/ORM/EntityManagerTest.php +++ b/tests/Doctrine/Tests/ORM/EntityManagerTest.php @@ -232,6 +232,24 @@ class EntityManagerTest extends OrmTestCase EntityManager::create(1, $config); } + /** + * @group #5796 + */ + public function testTransactionalReThrowsThrowables() + { + try { + $this->_em->transactional(function () { + (function (array $value) { + // this only serves as an IIFE that throws a `TypeError` + })(null); + }); + + self::fail('TypeError expected to be thrown'); + } catch (\TypeError $ignored) { + self::assertFalse($this->_em->isOpen()); + } + } + /** * @group 6017 */