Updater::install() crashes on file transfer exceptions.

Bug introduced a few weeks ago in #2055851: Remove translation of exception messages.

@@ -253,7 +253,7 @@ public function update(&$filetransfer, $overrides = []) {
       return $this->postUpdateTasks();
     }
     catch (FileTransferException $e) {
-      throw new UpdaterFileTransferException(t('File Transfer failed, reason: @reason', ['@reason' => strtr($e->getMessage(), $e->arguments)]));
+      throw new UpdaterFileTransferException("File Transfer failed, reason: '" . strtr($e->getMessage(), $e->arguments) . "'");
     }
   }
 
@@ -291,7 +291,7 @@ public function install(&$filetransfer, $overrides = []) {
       return $this->postInstallTasks();
     }
     catch (FileTransferException $e) {
-      throw new UpdaterFileTransferException(t('File Transfer failed, reason: @reason', ['@reason' => strtr($e->getMessage(), $e->arguments)]));
+      throw new UpdaterFileTransferException("File Transfer failed, reason: '" . strtr($e->getMessage(), $e->arguments)) . "'";
     }
   }

The first of these changes is correct. The second is parenthesized incorrectly. The result is that the exception is concatenated with a string, therefore converted to a string, which then cannot be thrown.

Trivial fix.

But I'm concerned that this slipped through. (1) Static code inspection in IDE marks it as an error, which means our automated code analysis is not thorough enough. (2) Evidently this code path isn't covered by tests either.

Introduced in version 8.9 https://git.drupalcode.org/project/drupal/commit/951478f

We should add a follow-up for the missing test coverage but going to commit this since it was introduced.

Committed and pushed 8596983c00 to 9.1.x and 806618e681 to 9.0.x and f323884b0b to 8.9.x. Thanks!

Hotfix commit makes sense but let's leave it open to ensure the tests issue gets filed. Thanks!

A unit test of the Drupal\Core\Updater\Updater class could probably cover most of this (using vfsStreamWrapper to simulate file operations).

Beside that, I wonder if there are ways we could get phpstan or codesniffer to check for type errors like this? Throwing an expression that isn't an object or an instance of throwable should have been flagged by the automated checks...

@cburschka can you create the follow-up issue to discuss this - then we can close this one.

Cheers!

Created #3151319: Test coverage for \Drupal\Core\Updater classes for the unit tests.

I don't know enough of the status of phpstan in DrupalCI to know if we need an issue for that or if it is something that is in already in progress...

@cburschka thanks - now we have the followup we can close this one.