Issue #3259810 by tedbow, kunal.sachdev: Validation errors are not logged on cron update attempts

package_manager/tests/src/Traits/PackageManagerBypassTestTrait.php

+<?php
+
+namespace Drupal\Tests\package_manager\Traits;
+
+/**
+ * Common functions for testing using the package_manager_bypass module.
+ */
+trait PackageManagerBypassTestTrait {
+
+  /**
+   * Asserts the number of times an update was staged.
+   *
+   * @param int $attempted_times
+   *   The expected number of times an update was staged.
+   */
+  private function assertUpdateStagedTimes(int $attempted_times): void {
+    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $beginner */
+    $beginner = $this->container->get('package_manager.beginner');
+    $this->assertCount($attempted_times, $beginner->getInvocationArguments());
+
+    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $stager */
+    $stager = $this->container->get('package_manager.stager');
+    // If an update was attempted, then there will be two calls to the stager:
+    // one to change the constraints in composer.json, and another to actually
+    // update the installed dependencies.
+    $this->assertCount($attempted_times * 2, $stager->getInvocationArguments());
+
+    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $committer */
+    $committer = $this->container->get('package_manager.committer');
+    $this->assertEmpty($committer->getInvocationArguments());
+  }
+
+}

src/CronUpdater.php

@@ -4,6 +4,7 @@ namespace Drupal\automatic_updates;
 
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Logger\LoggerChannelFactoryInterface;
+use Drupal\package_manager\Exception\StageValidationException;
 
 /**
  * Defines a service that updates via cron.
@@ -114,6 +115,10 @@ class CronUpdater extends Updater {
       $this->apply();
       $this->destroy();
     }
+    catch (StageValidationException $e) {
+      $this->logger->error($this->getLogMessageForValidationException($e));
+      return;
+    }
     catch (\Throwable $e) {
       $this->logger->error($e->getMessage());
       return;
@@ -128,4 +133,31 @@ class CronUpdater extends Updater {
     );
   }
 
+  /**
+   * Generates a log message from a stage validation exception.
+   *
+   * @param \Drupal\package_manager\Exception\StageValidationException $exception
+   *   The validation exception.
+   *
+   * @return string
+   *   The formatted log message, including all the validation results.
+   */
+  protected function getLogMessageForValidationException(StageValidationException $exception): string {
+    $log_message = '';
+    foreach ($exception->getResults() as $result) {
+      $summary = $result->getSummary();
+      if ($summary) {
+        $log_message .= "<h3>$summary</h3><ul>";
+        foreach ($result->getMessages() as $message) {
+          $log_message .= "<li>$message</li>";
+        }
+        $log_message .= "</ul>";
+      }
+      else {
+        $log_message .= ($log_message ? ' ' : '') . $result->getMessages()[0];
+      }
+    }
+    return "<h2>{$exception->getMessage()}</h2>$log_message";
+  }
+
 }

tests/src/Functional/UpdaterFormTest.php

@@ -6,6 +6,7 @@ use Drupal\automatic_updates\Event\ReadinessCheckEvent;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\ValidationResult;
 use Drupal\automatic_updates_test\ReadinessChecker\TestChecker1;
+use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait;
 
 /**
@@ -16,6 +17,7 @@ use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait;
 class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
 
   use ValidationTestTrait;
+  use PackageManagerBypassTestTrait;
 
   /**
    * {@inheritdoc}
@@ -268,27 +270,4 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $assert_session->buttonExists('Continue');
   }
 
-  /**
-   * Asserts the number of times an update was staged.
-   *
-   * @param int $attempted_times
-   *   The expected number of times an update was staged.
-   */
-  private function assertUpdateStagedTimes(int $attempted_times): void {
-    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $beginner */
-    $beginner = $this->container->get('package_manager.beginner');
-    $this->assertCount($attempted_times, $beginner->getInvocationArguments());
-
-    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $stager */
-    $stager = $this->container->get('package_manager.stager');
-    // If an update was attempted, then there will be two calls to the stager:
-    // one to change the constraints in composer.json, and another to actually
-    // update the installed dependencies.
-    $this->assertCount($attempted_times * 2, $stager->getInvocationArguments());
-
-    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $committer */
-    $committer = $this->container->get('package_manager.committer');
-    $this->assertEmpty($committer->getInvocationArguments());
-  }
-
 }

tests/src/Kernel/CronUpdaterTest.php

@@ -3,8 +3,14 @@
 namespace Drupal\Tests\automatic_updates\Kernel;
 
 use Drupal\automatic_updates\CronUpdater;
+use Drupal\automatic_updates_test\ReadinessChecker\TestChecker1;
 use Drupal\Core\Form\FormState;
+use Drupal\Core\Logger\RfcLogLevel;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\ValidationResult;
+use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 use Drupal\update\UpdateSettingsForm;
+use Psr\Log\Test\TestLogger;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 /**
@@ -15,6 +21,8 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
  */
 class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
 
+  use PackageManagerBypassTestTrait;
+
   /**
    * {@inheritdoc}
    */
@@ -22,6 +30,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     'automatic_updates',
     'package_manager',
     'package_manager_bypass',
+    'automatic_updates_test',
   ];
 
   /**
@@ -117,4 +126,67 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     $this->assertCount($will_update, $this->container->get('package_manager.committer')->getInvocationArguments());
   }
 
+  /**
+   * Data provider for testErrors().
+   *
+   * @return array[]
+   *   The test cases for testErrors().
+   */
+  public function providerErrors(): array {
+    $messages = [
+      'Precreate Event Error',
+      'Precreate Event Error 2',
+    ];
+    $summary = 'There were errors in updates';
+    $result_no_summary = ValidationResult::createError([$messages[0]]);
+    $result_with_summary = ValidationResult::createError($messages, t($summary));
+    $result_with_summary_message = "<h3>{$summary}</h3><ul><li>{$messages[0]}</li><li>{$messages[1]}</li></ul>";
+
+    return [
+      '1 result with summary' => [
+        [$result_with_summary],
+        $result_with_summary_message,
+      ],
+      '2 results with summary' => [
+        [$result_with_summary, $result_with_summary],
+        "$result_with_summary_message$result_with_summary_message",
+      ],
+      '1 result without summary' => [
+        [$result_no_summary],
+        $messages[0],
+      ],
+      '2 results without summary' => [
+        [$result_no_summary, $result_no_summary],
+        $messages[0] . ' ' . $messages[0],
+      ],
+      '1 result with summary, 1 result without summary' => [
+        [$result_with_summary, $result_no_summary],
+        $result_with_summary_message . ' ' . $messages[0],
+      ],
+    ];
+  }
+
+  /**
+   * Tests errors during a cron update attempt.
+   *
+   * @param \Drupal\package_manager\ValidationResult[] $validation_results
+   *   The expected validation results which should be logged.
+   * @param string $expected_log_message
+   *   The error message should be logged.
+   *
+   * @dataProvider providerErrors
+   */
+  public function testErrors(array $validation_results, string $expected_log_message): void {
+    TestChecker1::setTestResult($validation_results, PreCreateEvent::class);
+
+    $logger = new TestLogger();
+    $this->container->get('logger.factory')
+      ->get('automatic_updates')
+      ->addLogger($logger);
+
+    $this->container->get('cron')->run();
+    $this->assertUpdateStagedTimes(0);
+    $this->assertTrue($logger->hasRecord("<h2>Unable to complete the update because of errors.</h2>$expected_log_message", RfcLogLevel::ERROR));
+  }
+
 }