$connection->startTransaction() calls should now be placed within the try block

[mglaman]

https://www.drupal.org/node/3318042

Introduced in branch/version: 9.5.x / 9.5.0

Because Drupal's SQLite database driver now acquires a write lock immediately when it begins a transaction, it is possible for the ->startTransaction() call itself to throw an exception if that lock can't be acquired within the timeout defined by PDO::ATTR_TIMEOUT.

Because of this, all of Drupal core's calls to ->startTransaction() have been moved to be inside the try block, and it is recommended for contrib modules that call ->startTransaction() to do the same.

Before:

$transaction = $connection->startTransaction();
try {
  // Execute queries.
  ...
}
catch (\Exception $e) {
  $transaction->rollBack();

  // Do something with the exception, such as log it and rethrow it.
  ...
}

After:

try {
  $transaction = $connection->startTransaction();

  // Execute queries.
  ...
}
catch (\Exception $e) {
  if (isset($transaction)) {
    $transaction->rollBack();
  }

  // Do something with the exception, such as log it and rethrow it.
  ...
}

As shown above, when moving the $transaction = $connection->startTransaction(); to inside the try block, the $transaction->rollBack(); statement in the catch block must be wrapped inside an if (isset($transaction)) block.

It's not necessarily too problematic if some modules don't move their $transaction = $connection->startTransaction(); statements into the try block. It would just mean that if an exception were thrown there, then the catch block wouldn't run, so if that catch block does something helpful like log that exception, then that helpful thing would not happen for the case that ->startTransaction() itself throws the exception.

[mglaman]

Can phpstan-drupal detect if this is outside of a try/catch and recommend it is in one?

[bbrala]

This is in their blogpost how you could do this: https://phpstan.org/blog/preprocessing-ast-for-custom-rules

Add a visitor that tracks when it enters a try, then the check is pretty easy to know if startTransaction is within such a block