feat(plugins): display errors when plugin is invalid instead of crashing

modules/Plugins/Core/BasePlugin.php

@@ -21,7 +21,6 @@ use Modules\Plugins\Manifest\Manifest;
 use Modules\Plugins\Manifest\Person;
 use Modules\Plugins\Manifest\Repository;
 use Modules\Plugins\Manifest\Settings;
-use RuntimeException;
 
 /**
  * @property string $key
@@ -33,7 +32,12 @@ abstract class BasePlugin implements PluginInterface
 
     protected string $iconSrc;
 
-    protected bool $active;
+    /**
+     * @var array<string,string>
+     */
+    protected array $errors = [];
+
+    protected PluginStatus $status;
 
     protected Manifest $manifest;
 
@@ -51,20 +55,30 @@ abstract class BasePlugin implements PluginInterface
         $manifestContents = file_get_contents($manifestPath);
 
         if (! $manifestContents) {
-            throw new RuntimeException(sprintf('Plugin manifest "%s" is missing!', $manifestPath));
+            $manifestContents = '{}';
+            $this->errors['manifest'] = lang('Plugins.errors.manifestMissing', [
+                'manifestPath' => $manifestPath,
+            ]);
         }
 
         /** @var array<mixed>|null $manifestData */
         $manifestData = json_decode($manifestContents, true);
 
         if ($manifestData === null) {
-            throw new RuntimeException(sprintf('Plugin manifest "%s" is not a valid JSON', $manifestPath), 1);
+            $manifestData = [];
+            $this->errors['manifest'] = lang('Plugins.errors.manifestJsonInvalid', [
+                'manifestPath' => $manifestPath,
+            ]);
         }
 
-        $this->manifest = new Manifest($manifestData);
+        $this->manifest = new Manifest($this->key, $manifestData);
+        $this->errors = [...$this->errors, ...Manifest::getPluginErrors($this->key)];
 
-        // check that plugin is active
-        $this->active = get_plugin_option($this->key, 'active') ?? false;
+        if ($this->errors !== []) {
+            $this->status = PluginStatus::INVALID;
+        } else {
+            $this->status = get_plugin_option($this->key, 'active') ? PluginStatus::ACTIVE : PluginStatus::INACTIVE;
+        }
 
         $this->iconSrc = $this->loadIcon($directory . '/icon.svg');
 
@@ -98,9 +112,17 @@ abstract class BasePlugin implements PluginInterface
     {
     }
 
-    final public function isActive(): bool
+    final public function getStatus(): PluginStatus
     {
-        return $this->active;
+        return $this->status;
+    }
+
+    /**
+     * @return array<string,string>
+     */
+    final public function getErrors(): array
+    {
+        return $this->errors;
     }
 
     final public function isHookDeclared(string $name): bool
@@ -144,19 +166,6 @@ abstract class BasePlugin implements PluginInterface
         return $this->iconSrc;
     }
 
-    final public function doesManifestHaveErrors(): bool
-    {
-        return $this->getManifestErrors() !== [];
-    }
-
-    /**
-     * @return array<string,string>
-     */
-    final public function getManifestErrors(): array
-    {
-        return $this->manifest::$errors;
-    }
-
     /**
      * @return Field[]
      */

modules/Plugins/Core/PluginStatus.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Modules\Plugins\Core;
+
+enum PluginStatus: string
+{
+    case INVALID = 'invalid';
+    case INACTIVE = 'inactive';
+    case ACTIVE = 'active';
+}

modules/Plugins/Core/Plugins.php

@@ -96,7 +96,7 @@ class Plugins
     {
         $activePlugins = [];
         foreach (static::$plugins as $plugin) {
-            if ($plugin->isActive()) {
+            if ($plugin->getStatus() === PluginStatus::ACTIVE) {
                 $activePlugins[] = $plugin;
             }
         }
@@ -111,7 +111,7 @@ class Plugins
     {
         $pluginsWithPodcastSettings = [];
         foreach (static::$plugins as $plugin) {
-            if (! $plugin->isActive()) {
+            if ($plugin->getStatus() !== PluginStatus::ACTIVE) {
                 continue;
             }
 
@@ -132,7 +132,7 @@ class Plugins
     {
         $pluginsWithEpisodeSettings = [];
         foreach (static::$plugins as $plugin) {
-            if (! $plugin->isActive()) {
+            if ($plugin->getStatus() !== PluginStatus::ACTIVE) {
                 continue;
             }
 
@@ -183,7 +183,7 @@ class Plugins
     {
         foreach (static::$plugins as $plugin) {
             // only run hook on active plugins
-            if (! $plugin->isActive()) {
+            if ($plugin->getStatus() !== PluginStatus::ACTIVE) {
                 continue;
             }
 
@@ -282,7 +282,7 @@ class Plugins
             static::$pluginsByVendor[$vendor][] = $plugin;
             ++static::$installedCount;
 
-            if ($plugin->isActive()) {
+            if ($plugin->getStatus() === PluginStatus::ACTIVE) {
                 ++static::$activeCount;
             }
         }

modules/Plugins/Language/en/Plugins.php

@@ -28,6 +28,7 @@ return [
     'deactivate' => 'Deactivate',
     'active' => 'Active',
     'inactive' => 'Inactive',
+    'invalid' => 'Invalid',
     'uninstall' => 'Uninstall',
     'keywords' => [
         'podcasting20' => 'Podcasting 2.0',
@@ -40,4 +41,9 @@ return [
     'messages' => [
         'saveSettingsSuccess' => '{pluginName} settings were successfully saved!',
     ],
+    'errors' => [
+        'manifestError' => 'Plugin manifest has errors',
+        'manifestMissing' => 'Plugin manifest "{manifestPath}" is missing.',
+        'manifestJsonInvalid' => 'Plugin manifest "{manifestPath}" is not a valid JSON.',
+    ],
 ];

modules/Plugins/Manifest/Manifest.php

@@ -46,9 +46,9 @@ class Manifest extends ManifestObject
         'repository' => Repository::class,
     ];
 
-    protected string $name;
+    protected ?string $name = '???';
 
-    protected string $version;
+    protected ?string $version = 'X.Y.Z';
 
     protected ?string $description = null;
 

modules/Plugins/Manifest/ManifestObject.php

@@ -17,16 +17,19 @@ abstract class ManifestObject
     protected const CASTS = [];
 
     /**
-     * @var array<string,string>
+     * @var array<string,array<string,string>>
      */
-    public static array $errors = [];
+    protected static array $errors = [];
 
     /**
      * @param mixed[] $data
      */
     public function __construct(
-        private readonly array $data
+        protected readonly string $pluginKey,
+        private readonly array $data,
     ) {
+        self::$errors[$pluginKey] = [];
+
         $this->load();
     }
 
@@ -52,7 +55,11 @@ abstract class ManifestObject
         $validation->setRules($this::VALIDATION_RULES);
 
         if (! $validation->run($this->data)) {
-            static::$errors = [...static::$errors, ...$validation->getErrors()];
+            foreach ($validation->getErrors() as $key => $message) {
+                $this->addError($key, $message);
+            }
+
+            $validation->reset();
         }
 
         foreach ($validation->getValidated() as $key => $value) {
@@ -62,11 +69,11 @@ abstract class ManifestObject
                 if (is_array($cast)) {
                     if (is_array($value)) {
                         foreach ($value as $valueKey => $valueElement) {
-                            $value[$valueKey] = new $cast[0]($valueElement);
+                            $value[$valueKey] = new $cast[0]($this->pluginKey, $valueElement);
                         }
                     }
                 } else {
-                    $value = new $cast($value);
+                    $value = new $cast($this->pluginKey, $value ?? []);
                 }
             }
 
@@ -77,8 +84,13 @@ abstract class ManifestObject
     /**
      * @return array<string,string>
      */
-    public function getErrors(): array
+    public static function getPluginErrors(string $pluginKey): array
     {
-        return $this->errors;
+        return self::$errors[$pluginKey];
+    }
+
+    protected function addError(string $errorKey, string $errorMessage): void
+    {
+        self::$errors[$this->pluginKey][$errorKey] = $errorMessage;
     }
 }

modules/Plugins/Manifest/Person.php

@@ -35,7 +35,7 @@ class Person extends ManifestObject
 
     protected ?URI $url = null;
 
-    public function __construct(array|string $data)
+    public function __construct(string $pluginKey, array|string $data)
     {
         if (is_string($data)) {
             $result = preg_match(self::AUTHOR_STRING_PATTERN, $data, $matches);
@@ -51,6 +51,6 @@ class Person extends ManifestObject
             ];
         }
 
-        parent::__construct($data);
+        parent::__construct($pluginKey, $data);
     }
 }