diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php index 379904cef2a9a981c056559b326ccfafbdfb54d9..2a53321dc5150e3635fec4acf604f4bfcfffc329 100644 --- a/package_manager/src/Stage.php +++ b/package_manager/src/Stage.php @@ -256,12 +256,20 @@ class Stage implements LoggerAwareInterface { * A list of paths that Composer Stager should ignore when creating the * stage directory and applying staged changes to the active directory. * + * @throws \Drupal\package_manager\Exception\StageException + * Thrown if an exception occurs while collecting ignored paths. + * * @see ::create() * @see ::apply() */ protected function getIgnoredPaths(): array { $event = new CollectIgnoredPathsEvent($this); - $this->eventDispatcher->dispatch($event); + try { + $this->eventDispatcher->dispatch($event); + } + catch (\Throwable $e) { + $this->rethrowAsStageException($e); + } return $event->getAll(); } @@ -283,7 +291,9 @@ class Stage implements LoggerAwareInterface { * as long as the stage needs to exist. * * @throws \Drupal\package_manager\Exception\StageException - * Thrown if a stage directory already exists. + * Thrown if a stage directory already exists, or if an error occurs while + * creating the stage directory. In the latter situation, the stage + * directory will be destroyed. * * @see ::claim() */ @@ -312,22 +322,32 @@ class Stage implements LoggerAwareInterface { $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot()); $stage_dir = $this->pathFactory->create($this->getStageDirectory()); - try { - $ignored_paths = $this->getIgnoredPaths(); - } - catch (\Throwable $throwable) { - throw new StageException($this, $throwable->getMessage(), $throwable->getCode(), $throwable); - } - $event = new PreCreateEvent($this, $ignored_paths); + $event = new PreCreateEvent($this, $this->getIgnoredPaths()); // If an error occurs and we won't be able to create the stage, mark it as // available. $this->dispatch($event, [$this, 'markAsAvailable']); - $this->beginner->begin($active_dir, $stage_dir, new PathList($event->getExcludedPaths()), NULL, $timeout); + try { + $this->beginner->begin($active_dir, $stage_dir, new PathList($event->getExcludedPaths()), NULL, $timeout); + } + catch (\Throwable $error) { + $this->destroy(); + $this->rethrowAsStageException($error); + } $this->dispatch(new PostCreateEvent($this)); return $id; } + /** + * Wraps an exception in a StageException and re-throws it. + * + * @param \Throwable $e + * The throwable to wrap. + */ + private function rethrowAsStageException(\Throwable $e): never { + throw new StageException($this, $e->getMessage(), $e->getCode(), $e); + } + /** * Adds or updates packages in the stage directory. * @@ -340,33 +360,56 @@ class Stage implements LoggerAwareInterface { * @param int|null $timeout * (optional) How long to allow the Composer operation to run before timing * out, in seconds, or NULL to never time out. Defaults to 300 seconds. + * + * @throws \Drupal\package_manager\Exception\StageException + * Thrown if the Composer operation cannot be started, or if an error occurs + * during the operation. In the latter situation, the stage directory will + * be destroyed. */ public function require(array $runtime, array $dev = [], ?int $timeout = 300): void { $this->checkOwnership(); $this->dispatch(new PreRequireEvent($this, $runtime, $dev)); - $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot()); - $stage_dir = $this->pathFactory->create($this->getStageDirectory()); + + // A helper function to execute a command in the stage, destroying it if an + // exception occurs in the middle of a Composer operation. + $do_stage = function (array $command) use ($timeout): void { + $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot()); + $stage_dir = $this->pathFactory->create($this->getStageDirectory()); + + try { + $this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout); + } + catch (\Throwable $e) { + // If the caught exception isn't InvalidArgumentException or + // PreconditionException, a Composer operation was actually attempted, + // and failed. The stage should therefore be destroyed, because it's in + // an indeterminate and possibly unrecoverable state. + if (!$e instanceof InvalidArgumentException && !$e instanceof PreconditionException) { + $this->destroy(); + } + $this->rethrowAsStageException($e); + } + }; // Change the runtime and dev requirements as needed, but don't update // the installed packages yet. if ($runtime) { self::validateRequirements($runtime); $command = array_merge(['require', '--no-update'], $runtime); - $this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout); + $do_stage($command); } if ($dev) { self::validateRequirements($dev); $command = array_merge(['require', '--dev', '--no-update'], $dev); - $this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout); + $do_stage($command); } // If constraints were changed, update those packages. if ($runtime || $dev) { $command = array_merge(['update', '--with-all-dependencies'], $runtime, $dev); - $this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout); + $do_stage($command); } - $this->dispatch(new PostRequireEvent($this, $runtime, $dev)); } @@ -397,17 +440,10 @@ class Stage implements LoggerAwareInterface { $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot()); $stage_dir = $this->pathFactory->create($this->getStageDirectory()); - try { - $ignored_paths = $this->getIgnoredPaths(); - } - catch (\Throwable $throwable) { - throw new StageException($this, $throwable->getMessage(), $throwable->getCode(), $throwable); - } - // If an error occurs while dispatching the events, ensure that ::destroy() // doesn't think we're in the middle of applying the staged changes to the // active directory. - $event = new PreApplyEvent($this, $ignored_paths); + $event = new PreApplyEvent($this, $this->getIgnoredPaths()); $this->tempStore->set(self::TEMPSTORE_APPLY_TIME_KEY, $this->time->getRequestTime()); $this->dispatch($event, $this->setNotApplying()); @@ -426,7 +462,7 @@ class Stage implements LoggerAwareInterface { // The commit operation has not started yet, so we can clear the failure // marker. $this->failureMarker->clear(); - throw new StageException($this, $e->getMessage(), $e->getCode(), $e); + $this->rethrowAsStageException($e); } catch (\Throwable $throwable) { // The commit operation may have failed midway through, and the site code diff --git a/package_manager/tests/modules/package_manager_bypass/src/ComposerStagerExceptionTrait.php b/package_manager/tests/modules/package_manager_bypass/src/ComposerStagerExceptionTrait.php new file mode 100644 index 0000000000000000000000000000000000000000..7284dd31a169b3f83b46be41cb39487433b4c06c --- /dev/null +++ b/package_manager/tests/modules/package_manager_bypass/src/ComposerStagerExceptionTrait.php @@ -0,0 +1,33 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\package_manager_bypass; + +/** + * Trait to make Composer Stager throw pre-determined exceptions in tests. + * + * @internal + */ +trait ComposerStagerExceptionTrait { + + /** + * Sets an exception to be thrown. + * + * @param \Throwable $exception + * The throwable. + */ + public static function setException(\Throwable $exception): void { + \Drupal::state()->set(static::class . '-exception', $exception); + } + + /** + * Throws the exception if set. + */ + protected function throwExceptionIfSet(): void { + if ($exception = $this->state->get(static::class . '-exception')) { + throw $exception; + } + } + +} diff --git a/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php b/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php index 83b15c61d115838e17ea10dd07659e0d832050a8..ffe51348c74c7d6fc013a3017f557d735651361d 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php +++ b/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php @@ -18,6 +18,7 @@ use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface; */ final class LoggingBeginner implements BeginnerInterface { + use ComposerStagerExceptionTrait; use LoggingDecoratorTrait; /** @@ -45,6 +46,7 @@ final class LoggingBeginner implements BeginnerInterface { */ public function begin(PathInterface $activeDir, PathInterface $stagingDir, ?PathListInterface $exclusions = NULL, ?ProcessOutputCallbackInterface $callback = NULL, ?int $timeout = ProcessRunnerInterface::DEFAULT_TIMEOUT): void { $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions, $timeout); + $this->throwExceptionIfSet(); $this->inner->begin($activeDir, $stagingDir, $exclusions, $callback, $timeout); } diff --git a/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php b/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php index bafb28811ffbe00a22bc70b96e7cc5e7321b743f..e6aea32a9dc50a8476c38943e0b3bc860c5bac36 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php +++ b/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php @@ -18,6 +18,7 @@ use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface; */ final class LoggingCommitter implements CommitterInterface { + use ComposerStagerExceptionTrait; use LoggingDecoratorTrait; /** @@ -45,20 +46,8 @@ final class LoggingCommitter 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->throwExceptionIfSet(); $this->inner->commit($stagingDir, $activeDir, $exclusions, $callback, $timeout); } - /** - * 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); - } - } diff --git a/package_manager/tests/modules/package_manager_bypass/src/NoOpStager.php b/package_manager/tests/modules/package_manager_bypass/src/NoOpStager.php index a0ad0af166c8b652f15ebc7114c23ab2ba32dbde..a7819d302eca1d4191d9e4ec41a2a47e1c8a9099 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/NoOpStager.php +++ b/package_manager/tests/modules/package_manager_bypass/src/NoOpStager.php @@ -27,6 +27,7 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface; */ final class NoOpStager implements StagerInterface { + use ComposerStagerExceptionTrait; use LoggingDecoratorTrait; /** @@ -44,6 +45,7 @@ final class NoOpStager implements StagerInterface { */ public function stage(array $composerCommand, PathInterface $activeDir, PathInterface $stagingDir, ?ProcessOutputCallbackInterface $callback = NULL, ?int $timeout = ProcessRunnerInterface::DEFAULT_TIMEOUT): void { $this->saveInvocationArguments($composerCommand, $stagingDir, $timeout); + $this->throwExceptionIfSet(); // If desired, simulate a change to the lock file (e.g., as a result of // running `composer update`). diff --git a/package_manager/tests/src/Kernel/StageTest.php b/package_manager/tests/src/Kernel/StageTest.php index 6c96779c1f4bbd1443139ecac5715cd171e498a0..de336bae5f95c39159da58e254d0a80a7c650c43 100644 --- a/package_manager/tests/src/Kernel/StageTest.php +++ b/package_manager/tests/src/Kernel/StageTest.php @@ -17,7 +17,9 @@ use Drupal\package_manager\Event\StageEvent; use Drupal\package_manager\Exception\ApplyFailedException; use Drupal\package_manager\Exception\StageException; use Drupal\package_manager\Exception\StageFailureMarkerException; +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\PreconditionException; use PhpTuf\ComposerStager\Domain\Service\Precondition\PreconditionInterface; @@ -544,6 +546,57 @@ class StageTest extends PackageManagerKernelTestBase { $this->assertFalse($logger->hasRecord($warning_message, LogLevel::WARNING)); } + /** + * Data provider for ::testFailureDuringComposerStagerOperations(). + * + * @return array[] + * The test cases. + */ + public function providerFailureDuringComposerStagerOperations(): array { + return [ + [LoggingBeginner::class], + [NoOpStager::class], + [LoggingCommitter::class], + ]; + } + + /** + * Tests when Composer Stager throws an exception during an operation. + * + * @param string $throwing_class + * The fully qualified name of the Composer Stager class that should throw + * an exception. It is expected to have a static ::setException() method, + * provided by \Drupal\package_manager_bypass\ComposerStagerExceptionTrait. + * + * @dataProvider providerFailureDuringComposerStagerOperations + */ + public function testFailureDuringComposerStagerOperations(string $throwing_class): void { + if ($throwing_class === LoggingCommitter::class) { + // If the committer throws an exception, Stage will always re-throw it + // with a predetermined failure message. + $expected_message = 'Staged changes failed to apply, and the site is in an indeterminate state. It is strongly recommended to restore the code and database from a backup.'; + } + else { + $expected_message = "$throwing_class is angry!"; + } + + $exception = new \Exception($expected_message, 1024); + $throwing_class::setException($exception); + + $stage = $this->createStage(); + try { + $stage->create(); + $stage->require(['ext-json:*']); + $stage->apply(); + $this->fail('Expected an exception to be thrown, but it was not.'); + } + catch (StageException $e) { + $this->assertSame($expected_message, $e->getMessage()); + $this->assertSame(1024, $e->getCode()); + $this->assertSame($exception, $e->getPrevious()); + } + } + /** * Tests that ignored paths are collected before create and apply. */