Issue #3354099 by omkar.podey, phenaproxima, tedbow, Wim Leers, yash.rode: Add...

Issue #3354099 by omkar.podey, phenaproxima, tedbow, Wim Leers, yash.rode: Add functional test that proves there is reasonable UX whenever Composer Stager operations have a hard failure

package_manager/tests/modules/package_manager_bypass/src/ComposerStagerExceptionTrait.php

@@ -14,10 +14,10 @@ trait ComposerStagerExceptionTrait {
   /**
    * Sets an exception to be thrown.
    *
-   * @param \Throwable $exception
-   *   The throwable.
+   * @param \Throwable|null $exception
+   *   The exception to throw, or NULL to delete a stored exception.
    */
-  public static function setException(\Throwable $exception): void {
+  public static function setException(?\Throwable $exception): void {
     \Drupal::state()->set(static::class . '-exception', $exception);
   }
 

src/BatchProcessor.php

@@ -95,11 +95,16 @@ final class BatchProcessor {
    */
   public static function stage(): void {
     $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY);
+    $stage = static::getStage();
     try {
-      static::getStage()->claim($stage_id)->stage();
+      $stage->claim($stage_id)->stage();
     }
     catch (\Throwable $e) {
-      static::clean($stage_id);
+      // If the stage was not already destroyed because of this exception
+      // destroy it.
+      if (!$stage->isAvailable()) {
+        static::clean($stage_id);
+      }
       static::storeErrorMessage($e->getMessage());
       throw $e;
     }

tests/src/Functional/ComposerStagerOperationFailureTest.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Tests\automatic_updates\Functional;
+
+use Drupal\package_manager_bypass\LoggingBeginner;
+use Drupal\package_manager_bypass\LoggingCommitter;
+use Drupal\package_manager_bypass\NoOpStager;
+use PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException;
+use PhpTuf\ComposerStager\Domain\Exception\LogicException;
+
+/**
+ * @covers \Drupal\automatic_updates\Form\UpdaterForm
+ * @group automatic_updates
+ * @internal
+ */
+class ComposerStagerOperationFailureTest extends UpdaterFormTestBase {
+
+  /**
+   * Tests Composer operation failure is handled properly.
+   *
+   * @param string $exception_class
+   *   The exception class.
+   * @param string $service_class
+   *   The Composer Stager service which should throw an exception.
+   *
+   * @dataProvider providerComposerOperationFailure
+   */
+  public function testComposerOperationFailure(string $exception_class, string $service_class): void {
+    $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1');
+
+    $page = $this->getSession()->getPage();
+    $assert_session = $this->assertSession();
+    $this->mockActiveCoreVersion('9.8.0');
+    $this->checkForUpdates();
+    $this->drupalGet('/admin/modules/update');
+    $page->hasButton('Update to 9.8.1');
+
+    // Make the specified Composer Stager operation class throw an exception.
+    $exception = new $exception_class('Failure from inside ' . $service_class);
+    call_user_func([$service_class, 'setException'], $exception);
+
+    // Start the update.
+    $page->pressButton('Update to 9.8.1');
+    $this->checkForMetaRefresh();
+    // We can't continue the update after an error in the committer.
+    if ($service_class === LoggingCommitter::class) {
+      $page->pressButton('Continue');
+      $this->checkForMetaRefresh();
+      $this->clickLink('the error page');
+      $assert_session->statusCodeEquals(200);
+      $assert_session->statusMessageContains('Automatic updates failed to apply, and the site is in an indeterminate state. Consider restoring the code and database from a backup.');
+      return;
+    }
+    $this->clickLink('the error page');
+    $assert_session->statusMessageContains($exception->getMessage());
+
+    // Make the same Composer Stager operation class NOT throw an exception.
+    call_user_func([$service_class, 'setException'], NULL);
+
+    // Set up the update to 9.8.1 again as the stage gets destroyed after an
+    // exception occurs.
+    $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1');
+    // Stage should be automatically deleted when an error occurs.
+    $assert_session->buttonNotExists('Delete existing update');
+    // This ensures that we can still update after the exception no longer
+    // exists.
+    $page->pressButton('Update to 9.8.1');
+    $this->checkForMetaRefresh();
+    $page->pressButton('Continue');
+    $this->checkForMetaRefresh();
+    $assert_session->statusMessageContains('Update complete!');
+  }
+
+  /**
+   * Data provider for testComposerOperationFailure().
+   *
+   * @return string[][]
+   *   The test cases.
+   */
+  public function providerComposerOperationFailure(): array {
+    return [
+      'LogicException from Beginner' => [LogicException::class, LoggingBeginner::class],
+      'LogicException from Stager' => [LogicException::class, NoOpStager::class],
+      'InvalidArgumentException from Stager' => [InvalidArgumentException::class, NoOpStager::class],
+      'LogicException from Committer' => [LogicException::class, LoggingCommitter::class],
+    ];
+  }
+
+}