Issue #3344583 by yash.rode, kunal.sachdev, phenaproxima, Wim Leers:...

Issue #3344583 by yash.rode, kunal.sachdev, phenaproxima, Wim Leers: ComposerInspector::validate() should throw ComposerNotReadyException instead of \Exception

package_manager/src/ComposerInspector.php

@@ -6,6 +6,7 @@ namespace Drupal\package_manager;
 
 use Composer\Semver\Semver;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\package_manager\Exception\ComposerNotReadyException;
 use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
 use PhpTuf\ComposerStager\Domain\Exception\RuntimeException;
 use PhpTuf\ComposerStager\Domain\Service\Precondition\ComposerIsAvailableInterface;
@@ -91,7 +92,7 @@ class ComposerInspector {
    * @param string $working_dir
    *   The directory to check.
    *
-   * @throws \Exception
+   * @throws \Drupal\package_manager\Exception\ComposerNotReadyException
    *   Thrown if:
    *   - `composer.json` doesn't exist in the given directory or is invalid
    *     according to `composer validate`.
@@ -99,6 +100,7 @@ class ComposerInspector {
    */
   private function validateProject(string $working_dir): void {
     $messages = [];
+    $previous_exception = NULL;
 
     // If either composer.json or composer.lock have changed, ensure the
     // directory is in a completely valid state, according to Composer.
@@ -118,6 +120,7 @@ class ComposerInspector {
       }
       catch (RuntimeException $e) {
         $messages[] = $e->getMessage();
+        $previous_exception = $e;
       }
     }
 
@@ -130,7 +133,7 @@ class ComposerInspector {
     }
 
     if ($messages) {
-      throw new \Exception(implode("\n", $messages));
+      throw new ComposerNotReadyException($working_dir, $messages, 0, $previous_exception);
     }
   }
 
@@ -200,7 +203,7 @@ class ComposerInspector {
     }
 
     if ($messages) {
-      throw new \Exception(implode("\n", $messages));
+      throw new ComposerNotReadyException(NULL, $messages);
     }
   }
 

package_manager/src/Exception/ComposerNotReadyException.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\package_manager\Exception;
+
+/**
+ * Exception thrown if we cannot reliably use Composer.
+ *
+ * Should not be thrown by external code.
+ *
+ * @see \Drupal\package_manager\ComposerInspector::validate()
+ */
+final class ComposerNotReadyException extends \RuntimeException {
+
+  /**
+   * Constructs an ComposerNotReadyException object.
+   *
+   * @param string|null $workingDir
+   *   The directory where Composer was run , or NULL if the errors are related
+   *   to the Composer executable itself.
+   * @param array $messages
+   *   An array of messages explaining why Composer cannot be run correctly.
+   * @param int $code
+   *   (optional) The exception code. Defaults to 0.
+   * @param \Throwable|null $previous
+   *   (optional) The previous exception, for exception chaining.
+   */
+  public function __construct(public readonly ?string $workingDir, array $messages, int $code = 0, ?\Throwable $previous = NULL) {
+    parent::__construct(implode("\n", $messages), $code, $previous);
+  }
+
+}

package_manager/tests/src/Kernel/ComposerInspectorTest.php

@@ -7,6 +7,7 @@ namespace Drupal\Tests\package_manager\Kernel;
 use Composer\Json\JsonFile;
 use Drupal\Component\Serialization\Json;
 use Drupal\package_manager\ComposerInspector;
+use Drupal\package_manager\Exception\ComposerNotReadyException;
 use Drupal\package_manager\InstalledPackage;
 use Drupal\package_manager\JsonProcessOutputCallback;
 use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
@@ -154,11 +155,13 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase {
       $inspector->validate($project_root);
       $this->fail('Expected an exception to be thrown, but it was not.');
     }
-    catch (\Throwable $e) {
+    catch (ComposerNotReadyException $e) {
+      $this->assertNull($e->workingDir);
       $this->assertSame("Well, that didn't work.", $e->getMessage());
     }
 
     // Call validate() again to ensure the precondition is called once.
+    $this->expectException(ComposerNotReadyException::class);
     $this->expectExceptionMessage("Well, that didn't work.");
     $inspector->validate($project_root);
   }
@@ -182,9 +185,15 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase {
     $file_path = $project_root . '/' . $filename;
     unlink($file_path);
 
-    $this->expectExceptionMessage("$filename not found");
-    $this->container->get('package_manager.composer_inspector')
-      ->validate($project_root);
+    /** @var \Drupal\package_manager\ComposerInspector $inspector */
+    $inspector = $this->container->get('package_manager.composer_inspector');
+    try {
+      $inspector->validate($project_root);
+    }
+    catch (ComposerNotReadyException $e) {
+      $this->assertSame($project_root, $e->workingDir);
+      $this->assertStringContainsString("$filename not found", $e->getMessage());
+    }
   }
 
   /**
@@ -260,11 +269,13 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase {
       // an exception message.
       $this->assertNull($expected_message, 'Expected an exception, but none was thrown.');
     }
-    catch (\Throwable $e) {
+    catch (ComposerNotReadyException $e) {
+      $this->assertNull($e->workingDir);
       $this->assertSame($expected_message, $e->getMessage());
     }
 
     if (isset($expected_message)) {
+      $this->expectException(ComposerNotReadyException::class);
       $this->expectExceptionMessage($expected_message);
     }
     $inspector->validate($project_root);
@@ -285,9 +296,16 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase {
     $data['prefer-stable'] = 'truthy';
     $file->write($data);
 
-    $this->expectExceptionMessage('composer.json" does not match the expected JSON schema');
-    $this->container->get('package_manager.composer_inspector')
-      ->validate($project_root);
+    try {
+      $this->container->get('package_manager.composer_inspector')
+        ->validate($project_root);
+      $this->fail('Expected an exception to be thrown, but it was not.');
+    }
+    catch (ComposerNotReadyException $e) {
+      $this->assertSame($project_root, $e->workingDir);
+      $this->assertStringContainsString('composer.json" does not match the expected JSON schema', $e->getMessage());
+      $this->assertStringContainsString('prefer-stable : String value found, but a boolean is required', $e->getPrevious()?->getMessage());
+    }
   }
 
   /**