From a3b6dfbb7b0588f53afcb59b6b338aa36d9715ec Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 16 Dec 2016 02:20:02 +0100 Subject: [PATCH 1/2] Catch `\Throwable` in critical places Added catch `\Throwable` to be compatible with PHP7. Added it in cases where object state needs to be kept consistent. Mainly on transactions but also some other places where some values are reset before exiting. Most of them could probably be refactored by using `finally` in 2.1, as that requires PHP 5.5. fixes #12619 --- docs/guide/db-active-record.md | 2 +- docs/guide/db-dao.md | 8 ++++---- framework/CHANGELOG.md | 2 +- framework/db/ActiveRecord.php | 9 +++++++++ framework/db/Connection.php | 17 ++++++++++++++--- framework/db/Migration.php | 27 +++++++++++++++++++-------- framework/db/Transaction.php | 2 +- 7 files changed, 49 insertions(+), 18 deletions(-) diff --git a/docs/guide/db-active-record.md b/docs/guide/db-active-record.md index e93a0f4b95..dd70f4c024 100644 --- a/docs/guide/db-active-record.md +++ b/docs/guide/db-active-record.md @@ -633,7 +633,7 @@ try { $customer->save(); // ...other DB operations... $transaction->commit(); -} catch(\Exception $e) { +} catch(\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 $transaction->rollBack(); throw $e; } diff --git a/docs/guide/db-dao.md b/docs/guide/db-dao.md index b344b11824..f8a207425b 100644 --- a/docs/guide/db-dao.md +++ b/docs/guide/db-dao.md @@ -336,7 +336,7 @@ try { $transaction->commit(); -} catch(\Exception $e) { +} catch(\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 $transaction->rollBack(); @@ -421,13 +421,13 @@ try { try { $db->createCommand($sql2)->execute(); $innerTransaction->commit(); - } catch (\Exception $e) { + } catch (\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 $innerTransaction->rollBack(); throw $e; } $outerTransaction->commit(); -} catch (\Exception $e) { +} catch (\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 $outerTransaction->rollBack(); throw $e; } @@ -570,7 +570,7 @@ try { $db->createCommand("UPDATE user SET username='demo' WHERE id=1")->execute(); $transaction->commit(); -} catch(\Exception $e) { +} catch(\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 $transaction->rollBack(); throw $e; } diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index afaa3fd00b..156edaf279 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -54,7 +54,7 @@ Yii Framework 2 Change Log - Enh #12145: Added `beforeCacheResponse` and `afterRestoreResponse` to `yii\filters\PageCache` to be more easily extendable (sergeymakinen) - Enh #12390: Avoid creating queries with false where condition (`0=1`) when fetching relational data (klimov-paul) - Enh #12399: Added `ActiveField::addAriaAttributes` property for `aria-required` and `aria-invalid` attributes rendering (Oxyaction, samdark) -- Enh #12619: Added catch `Throwable` in `yii\base\ErrorHandler::handleException()` (rob006) +- Enh #12619: Added catch `Throwable` in `yii\base\ErrorHandler::handleException()`, transactions and simlar places where consistency must be kept after exception (rob006, cebe) - Enh #12659: Suggest alternatives when console command was not found (mdmunir, cebe) - Enh #12726: `yii\base\Application::$version` converted to `yii\base\Module::$version` virtual property, allowing to specify version as a PHP callback (klimov-paul) - Enh #12732: Added `is_dir()` validation to `yii\helpers\BaseFileHelper::findFiles()` method (zalatov, silverfire) diff --git a/framework/db/ActiveRecord.php b/framework/db/ActiveRecord.php index d403914bc4..06c67c4b94 100644 --- a/framework/db/ActiveRecord.php +++ b/framework/db/ActiveRecord.php @@ -439,6 +439,9 @@ class ActiveRecord extends BaseActiveRecord } catch (\Exception $e) { $transaction->rollBack(); throw $e; + } catch (\Throwable $e) { + $transaction->rollBack(); + throw $e; } } @@ -545,6 +548,9 @@ class ActiveRecord extends BaseActiveRecord } catch (\Exception $e) { $transaction->rollBack(); throw $e; + } catch (\Throwable $e) { + $transaction->rollBack(); + throw $e; } } @@ -585,6 +591,9 @@ class ActiveRecord extends BaseActiveRecord } catch (\Exception $e) { $transaction->rollBack(); throw $e; + } catch (\Throwable $e) { + $transaction->rollBack(); + throw $e; } } diff --git a/framework/db/Connection.php b/framework/db/Connection.php index 506431bac1..c7906a0ef4 100644 --- a/framework/db/Connection.php +++ b/framework/db/Connection.php @@ -420,7 +420,7 @@ class Connection extends Component * Use 0 to indicate that the cached data will never expire. * @param \yii\caching\Dependency $dependency the cache dependency associated with the cached query results. * @return mixed the return result of the callable - * @throws \Exception if there is any exception during query + * @throws \Exception|\Throwable if there is any exception during query * @see enableQueryCache * @see queryCache * @see noCache() @@ -435,6 +435,9 @@ class Connection extends Component } catch (\Exception $e) { array_pop($this->_queryCacheInfo); throw $e; + } catch (\Throwable $e) { + array_pop($this->_queryCacheInfo); + throw $e; } } @@ -457,7 +460,7 @@ class Connection extends Component * @param callable $callable a PHP callable that contains DB queries which should not use query cache. * The signature of the callable is `function (Connection $db)`. * @return mixed the return result of the callable - * @throws \Exception if there is any exception during query + * @throws \Exception|\Throwable if there is any exception during query * @see enableQueryCache * @see queryCache * @see cache() @@ -472,6 +475,9 @@ class Connection extends Component } catch (\Exception $e) { array_pop($this->_queryCacheInfo); throw $e; + } catch (\Throwable $e) { + array_pop($this->_queryCacheInfo); + throw $e; } } @@ -671,7 +677,7 @@ class Connection extends Component * @param callable $callback a valid PHP callback that performs the job. Accepts connection instance as parameter. * @param string|null $isolationLevel The isolation level to use for this transaction. * See [[Transaction::begin()]] for details. - * @throws \Exception + * @throws \Exception|\Throwable if there is any exception during query. In this case the transaction will be rolled back. * @return mixed result of callback function */ public function transaction(callable $callback, $isolationLevel = null) @@ -689,6 +695,11 @@ class Connection extends Component $transaction->rollBack(); } throw $e; + } catch (\Throwable $e) { + if ($transaction->isActive && $transaction->level === $level) { + $transaction->rollBack(); + } + throw $e; } return $result; diff --git a/framework/db/Migration.php b/framework/db/Migration.php index 7b5a4e1678..b642b66cbb 100644 --- a/framework/db/Migration.php +++ b/framework/db/Migration.php @@ -95,15 +95,16 @@ class Migration extends Component implements MigrationInterface try { if ($this->safeUp() === false) { $transaction->rollBack(); - return false; } $transaction->commit(); } catch (\Exception $e) { - echo 'Exception: ' . $e->getMessage() . ' (' . $e->getFile() . ':' . $e->getLine() . ")\n"; - echo $e->getTraceAsString() . "\n"; + $this->printException($e); + $transaction->rollBack(); + return false; + } catch (\Throwable $e) { + $this->printException($e); $transaction->rollBack(); - return false; } @@ -123,21 +124,31 @@ class Migration extends Component implements MigrationInterface try { if ($this->safeDown() === false) { $transaction->rollBack(); - return false; } $transaction->commit(); } catch (\Exception $e) { - echo 'Exception: ' . $e->getMessage() . ' (' . $e->getFile() . ':' . $e->getLine() . ")\n"; - echo $e->getTraceAsString() . "\n"; + $this->printException($e); + $transaction->rollBack(); + return false; + } catch (\Throwable $e) { + $this->printException($e); $transaction->rollBack(); - return false; } return null; } + /** + * @param \Throwable|\Exception $e + */ + private function printException($e) + { + echo 'Exception: ' . $e->getMessage() . ' (' . $e->getFile() . ':' . $e->getLine() . ")\n"; + echo $e->getTraceAsString() . "\n"; + } + /** * This method contains the logic to be executed when applying this migration. * This method differs from [[up()]] in that the DB logic implemented here will diff --git a/framework/db/Transaction.php b/framework/db/Transaction.php index d2877163bb..9f4e9da1ab 100644 --- a/framework/db/Transaction.php +++ b/framework/db/Transaction.php @@ -25,7 +25,7 @@ use yii\base\InvalidConfigException; * $connection->createCommand($sql2)->execute(); * //.... other SQL executions * $transaction->commit(); - * } catch (\Exception $e) { + * } catch (\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 * $transaction->rollBack(); * throw $e; * } From cb52c42cf50bea0bc02debca06b2d57aad954034 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Sat, 17 Dec 2016 00:43:48 +0100 Subject: [PATCH 2/2] more details about PHP 5 and 7 compatibility regarding Throwable --- docs/guide/db-active-record.md | 9 ++++++++- docs/guide/db-dao.md | 28 ++++++++++++++++++++-------- framework/db/Transaction.php | 9 ++++++++- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/docs/guide/db-active-record.md b/docs/guide/db-active-record.md index dd70f4c024..4a7b08e6cd 100644 --- a/docs/guide/db-active-record.md +++ b/docs/guide/db-active-record.md @@ -633,12 +633,19 @@ try { $customer->save(); // ...other DB operations... $transaction->commit(); -} catch(\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 +} catch(\Exception $e) { + $transaction->rollBack(); + throw $e; +} catch(\Throwable $e) { $transaction->rollBack(); throw $e; } ``` +> Note: in the above code we have two catch-blocks for compatibility +> with PHP 5.x and PHP 7.x. `\Exception` implements the [`\Throwable` interface](http://php.net/manual/en/class.throwable.php) +> since PHP 7.0, so you can skip the part with `\Exception` if your app uses only PHP 7.0 and higher. + The second way is to list the DB operations that require transactional support in the [[yii\db\ActiveRecord::transactions()]] method. For example, diff --git a/docs/guide/db-dao.md b/docs/guide/db-dao.md index f8a207425b..a4402a2891 100644 --- a/docs/guide/db-dao.md +++ b/docs/guide/db-dao.md @@ -328,18 +328,17 @@ The above code is equivalent to the following, which gives you more control abou ```php $db = Yii::$app->db; $transaction = $db->beginTransaction(); - try { $db->createCommand($sql1)->execute(); $db->createCommand($sql2)->execute(); // ... executing other SQL statements ... $transaction->commit(); - -} catch(\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 - +} catch(\Exception $e) { + $transaction->rollBack(); + throw $e; +} catch(\Throwable $e) { $transaction->rollBack(); - throw $e; } ``` @@ -352,6 +351,10 @@ will be triggered and caught, the [[yii\db\Transaction::rollBack()|rollBack()]] the changes made by the queries prior to that failed query in the transaction. `throw $e` will then re-throw the exception as if we had not caught it, so the normal error handling process will take care of it. +> Note: in the above code we have two catch-blocks for compatibility +> with PHP 5.x and PHP 7.x. `\Exception` implements the [`\Throwable` interface](http://php.net/manual/en/class.throwable.php) +> since PHP 7.0, so you can skip the part with `\Exception` if your app uses only PHP 7.0 and higher. + ### Specifying Isolation Levels @@ -421,13 +424,19 @@ try { try { $db->createCommand($sql2)->execute(); $innerTransaction->commit(); - } catch (\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 + } catch (\Exception $e) { + $innerTransaction->rollBack(); + throw $e; + } catch (\Throwable $e) { $innerTransaction->rollBack(); throw $e; } $outerTransaction->commit(); -} catch (\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 +} catch (\Exception $e) { + $outerTransaction->rollBack(); + throw $e; +} catch (\Throwable $e) { $outerTransaction->rollBack(); throw $e; } @@ -570,7 +579,10 @@ try { $db->createCommand("UPDATE user SET username='demo' WHERE id=1")->execute(); $transaction->commit(); -} catch(\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 +} catch(\Exception $e) { + $transaction->rollBack(); + throw $e; +} catch(\Throwable $e) { $transaction->rollBack(); throw $e; } diff --git a/framework/db/Transaction.php b/framework/db/Transaction.php index 9f4e9da1ab..85aca05fd3 100644 --- a/framework/db/Transaction.php +++ b/framework/db/Transaction.php @@ -25,12 +25,19 @@ use yii\base\InvalidConfigException; * $connection->createCommand($sql2)->execute(); * //.... other SQL executions * $transaction->commit(); - * } catch (\Exception $e) { // replace \Exception with \Throwable when you are using PHP 7 + * } catch (\Exception $e) { + * $transaction->rollBack(); + * throw $e; + * } catch (\Throwable $e) { * $transaction->rollBack(); * throw $e; * } * ``` * + * > Note: in the above code we have two catch-blocks for compatibility + * > with PHP 5.x and PHP 7.x. `\Exception` implements the [`\Throwable` interface](http://php.net/manual/en/class.throwable.php) + * > since PHP 7.0, so you can skip the part with `\Exception` if your app uses only PHP 7.0 and higher. + * * @property bool $isActive Whether this transaction is active. Only an active transaction can [[commit()]] * or [[rollBack()]]. This property is read-only. * @property string $isolationLevel The transaction isolation level to use for this transaction. This can be