Commit 51bf737d authored by Ted Bowman's avatar Ted Bowman Committed by Adam G-H
Browse files

Issue #3291770 by tedbow, kunal.sachdev, phenaproxima: Inform the site admin...

Issue #3291770 by tedbow, kunal.sachdev, phenaproxima: Inform the site admin if Composer Stager's committer failed, possibly leaving the site in a half-updated state
parent b0c632e9
Loading
Loading
Loading
Loading
+16 −0
Original line number Diff line number Diff line
<?php

namespace Drupal\package_manager\Exception;

/**
 * Exception thrown if a stage encounters an error applying an update.
 *
 * If this exception is thrown it indicates that an update of the active
 * codebase was attempted but failed. If this happens the site code is in an
 * indeterminate state. Package Manager does not provide a method for recovering
 * from this state. The site code should be restored from a backup.
 *
 * Should not be thrown by external code.
 */
final class ApplyFailedException extends StageException {
}
+23 −1
Original line number Diff line number Diff line
@@ -18,12 +18,15 @@ use Drupal\package_manager\Event\PreDestroyEvent;
use Drupal\package_manager\Event\PreOperationStageEvent;
use Drupal\package_manager\Event\PreRequireEvent;
use Drupal\package_manager\Event\StageEvent;
use Drupal\package_manager\Exception\ApplyFailedException;
use Drupal\package_manager\Exception\StageException;
use Drupal\package_manager\Exception\StageOwnershipException;
use Drupal\package_manager\Exception\StageValidationException;
use PhpTuf\ComposerStager\Domain\Core\Beginner\BeginnerInterface;
use PhpTuf\ComposerStager\Domain\Core\Committer\CommitterInterface;
use PhpTuf\ComposerStager\Domain\Core\Stager\StagerInterface;
use PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException;
use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
use PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactory;
use PhpTuf\ComposerStager\Infrastructure\Value\PathList\PathList;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
@@ -351,6 +354,10 @@ class Stage {
   *   (optional) How long to allow the file copying operation to run before
   *   timing out, in seconds, or NULL to never time out. Defaults to 600
   *   seconds.
   *
   * @throws \Drupal\package_manager\Exception\ApplyFailedException
   *   Thrown if there is an error calling Composer Stager, which may indicate
   *   a failed commit operation.
   */
  public function apply(?int $timeout = 600): void {
    $this->checkOwnership();
@@ -365,8 +372,23 @@ class Stage {
    $this->tempStore->set(self::TEMPSTORE_APPLY_TIME_KEY, $this->time->getRequestTime());
    $this->dispatch($event, $this->setNotApplying());

    try {
      $this->committer->commit($stage_dir, $active_dir, new PathList($event->getExcludedPaths()), NULL, $timeout);
    }
    // @todo When this module requires PHP 8 consolidate the next 2 catch blocks
    //   into 1.
    catch (InvalidArgumentException $e) {
      throw new StageException($e->getMessage(), $e->getCode(), $e);
    }
    catch (PreconditionException $e) {
      throw new StageException($e->getMessage(), $e->getCode(), $e);
    }
    catch (\Throwable $throwable) {
      // If the throwable is any other type the commit operation may have
      // failed.
      throw new ApplyFailedException($throwable->getMessage(), $throwable->getCode(), $throwable);
    }
  }

  /**
   * Returns a closure that marks this stage as no longer being applied.
+13 −0
Original line number Diff line number Diff line
@@ -18,6 +18,9 @@ class Committer extends BypassedStagerServiceBase implements CommitterInterface
   */
  public function commit(PathInterface $stagingDir, PathInterface $activeDir, ?PathListInterface $exclusions = NULL, ?ProcessOutputCallbackInterface $callback = NULL, ?int $timeout = ProcessRunnerInterface::DEFAULT_TIMEOUT): void {
    $this->saveInvocationArguments($stagingDir, $activeDir, $exclusions, $timeout);
    if ($exception = $this->state->get(static::class . '-exception')) {
      throw $exception;
    }
    $this->copyFixtureFilesTo($activeDir);
  }

@@ -31,4 +34,14 @@ class Committer extends BypassedStagerServiceBase implements CommitterInterface
    throw new \BadMethodCallException('This is not implemented yet.');
  }

  /**
   * Sets an exception to be thrown during ::commit().
   *
   * @param \Throwable $exception
   *   The throwable.
   */
  public static function setException(\Throwable $exception): void {
    \Drupal::state()->set(static::class . '-exception', $exception);
  }

}
+61 −0
Original line number Diff line number Diff line
@@ -10,8 +10,13 @@ use Drupal\Core\Site\Settings;
use Drupal\package_manager\Event\PostApplyEvent;
use Drupal\package_manager\Event\PreApplyEvent;
use Drupal\package_manager\Event\StageEvent;
use Drupal\package_manager\Exception\ApplyFailedException;
use Drupal\package_manager\Exception\StageException;
use Drupal\package_manager_bypass\Committer;
use PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException;
use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
use Drupal\package_manager_bypass\Beginner;
use PhpTuf\ComposerStager\Domain\Service\Precondition\PreconditionInterface;

/**
 * @coversDefaultClass \Drupal\package_manager\Stage
@@ -260,6 +265,62 @@ class StageTest extends PackageManagerKernelTestBase {
    }
  }

  /**
   * Data provider for testCommitException().
   *
   * @return \string[][]
   *   The test cases.
   */
  public function providerCommitException(): array {
    return [
      'RuntimeException to ApplyFailedException' => [
        'RuntimeException',
        ApplyFailedException::class,
      ],
      'InvalidArgumentException' => [
        InvalidArgumentException::class,
        StageException::class,
      ],
      'PreconditionException' => [
        PreconditionException::class,
        StageException::class,
      ],
      'Exception' => [
        'Exception',
        ApplyFailedException::class,
      ],
    ];
  }

  /**
   * Tests exception handling during calls to Composer Stager commit.
   *
   * @param string $thrown_class
   *   The throwable class that should be thrown by Composer Stager.
   * @param string|null $expected_class
   *   The expected exception class, if different from $thrown_class.
   *
   * @dataProvider providerCommitException
   */
  public function testCommitException(string $thrown_class, string $expected_class = NULL): void {
    $stage = $this->createStage();
    $stage->create();
    $thrown_message = 'A very bad thing happened';
    // PreconditionException requires a preconditions object.
    if ($thrown_class === PreconditionException::class) {
      $throwable = new PreconditionException($this->prophesize(PreconditionInterface::class)->reveal(), $thrown_message, 123);
    }
    else {
      $throwable = new $thrown_class($thrown_message, 123);
    }
    Committer::setException($throwable);
    $this->expectException($expected_class);
    $this->expectExceptionMessage($thrown_message);
    $this->expectExceptionCode(123);
    $stage->require(['drupal/core' => '9.8.1']);
    $stage->apply();
  }

}

/**
+1 −1
Original line number Diff line number Diff line
@@ -264,7 +264,7 @@ final class UpdaterForm extends FormBase {
    }

    $form['backup'] = [
      '#markup' => $this->t('It\'s a good idea to <a href=":url">back up your database</a> before you begin.', [':url' => 'https://www.drupal.org/node/22281#s-backing-up-the-database']),
      '#markup' => $this->t('It\'s a good idea to <a href=":url">back up your database and site code</a> before you begin.', [':url' => 'https://www.drupal.org/node/22281']),
    ];

    if ($stage_exists) {
Loading