Skip to content

Commit

Permalink
Merge pull request #10798 from jonasraoni/feature-main-10514-plugin-r…
Browse files Browse the repository at this point in the history 
…esilience

Improve plugin resilience
  • Loading branch information
@jonasraoni
jonasraoni authored Jan 16, 2025
2 parents c76f5d4 + c2521bb commit 1b31467
Show file tree
Hide file tree
Showing 3 changed files with 228 additions and 28 deletions.
60 changes: 50 additions & 10 deletions classes/plugins/Hook.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
namespace PKP\plugins;

use PKP\core\Registry;
use ReflectionClass;
use ReflectionFunction;
use Throwable;

class Hook
{
Expand Down Expand Up @@ -60,7 +63,7 @@ public static function &getHooks(?string $hookName = null): ?array
*/
public static function addUnsupportedHooks(...$hookNames): void
{
self::$unsupportedHooks = array_merge(self::$unsupportedHooks, array_flip($hookNames));
static::$unsupportedHooks = array_merge(static::$unsupportedHooks, array_flip($hookNames));
}

/**
Expand All @@ -81,7 +84,7 @@ public static function clear(string $hookName): void
*/
public static function add(string $hookName, callable $callback, int $hookSequence = self::SEQUENCE_NORMAL): void
{
if (isset(self::$unsupportedHooks[$hookName])) {
if (isset(static::$unsupportedHooks[$hookName])) {
throw new \Exception("Hook {$hookName} is not supported (possibly removed) and callbacks should not be added to it!");
}
$hooks = & static::getHooks();
Expand All @@ -96,7 +99,7 @@ public static function add(string $hookName, callable $callback, int $hookSequen
*/
public static function register(string $hookName, callable $callback, int $hookSequence = self::SEQUENCE_NORMAL): void
{
self::add($hookName, $callback, $hookSequence);
static::add($hookName, $callback, $hookSequence);
}

/**
Expand All @@ -118,15 +121,15 @@ public static function call(string $hookName, array $args = []): mixed
// Called only by Unit Test
// This behaviour is DEPRECATED and not replicated in the preferred
// Hook::call function.
if (self::rememberCalledHooks(true)) {
if (static::rememberCalledHooks(true)) {
// Remember the called hooks for testing.
$calledHooks = & static::getCalledHooks();
$calledHooks[] = [
$hookName, $args
];
}

return self::run($hookName, [$args]);
return static::run($hookName, [$args]);
}

/**
Expand All @@ -146,7 +149,7 @@ public static function run(string $hookName, array $args = []): bool
{
$hooks = & static::getHooks();
if (!isset($hooks[$hookName])) {
return self::CONTINUE;
return static::CONTINUE;
}

// Sort callbacks if the list is dirty
Expand All @@ -155,15 +158,52 @@ public static function run(string $hookName, array $args = []): bool
$hooks[$hookName]['dirty'] = false;
}

foreach ($hooks[$hookName]['hooks'] as $priority => $hookList) {
foreach ($hooks[$hookName]['hooks'] as $hookList) {
foreach ($hookList as $callback) {
if (call_user_func_array($callback, [$hookName, ...$args]) === self::ABORT) {
return self::ABORT;
try {
if (call_user_func_array($callback, [$hookName, ...$args]) === static::ABORT) {
return static::ABORT;
}
} catch (Throwable $e) {
/**
* This is an attempt to improve the application's resilience against errors inside plugins
* If an exception happens at a hook handler which was implemented inside a plugin (callbacks implemented inside a plugin-derived class or located inside the folders "lib/pkp/plugins" or "plugins"), the exception will be logged and the hook flow will continue
* @see https://github.com/pkp/pkp-lib/discussions/9083
*/
$pluginDirectories = [realpath(BASE_SYS_DIR . '/' . PKP_LIB_PATH . '/plugins'), realpath(BASE_SYS_DIR . '/plugins')];
foreach ($e->getTrace() as $stackFrame) {
// If the code was implemented inside a plugin class, let the hook flow continue
if (is_subclass_of($class = $stackFrame['class'] ?? null, Plugin::class)) {
error_log("Plugin {$class} failed to handle the hook {$hookName}\n{$e}");
continue 2;
}

// Attempt to recover the file where the callback was implemented
$filename = ($class ? (new ReflectionClass($class))->getFileName() : null)
?? (($function = $stackFrame['function'] ?? null) ? (new ReflectionFunction($function))->getFileName() : null)
?? $stackFrame['file']
?? null;
if (!$filename) {
continue;
}

// If the code was implemented inside a plugin folder, let the hook flow continue
$filename = realpath($filename);
foreach ($pluginDirectories as $pluginDirectory) {
if (strpos($filename, $pluginDirectory) === 0) {
$pieces = explode(DIRECTORY_SEPARATOR, substr($filename, strlen($pluginDirectory) + 1));
error_log("Plugin {$pieces[0]}/{$pieces[1]} failed to handle the hook {$hookName}\n{$e}");
continue 3;
}
}
}

throw $e;
}
}
}

return self::CONTINUE;
return static::CONTINUE;
}


Expand Down
68 changes: 50 additions & 18 deletions classes/plugins/PluginRegistry.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Illuminate\Support\Arr;
use PKP\core\Registry;
use ReflectionObject;
use Throwable;

class PluginRegistry
{
Expand Down Expand Up @@ -69,11 +70,23 @@ public static function getAllPlugins(): array
*/
public static function register(string $category, Plugin $plugin, string $path, ?int $mainContextId = null): bool
{
$pluginName = $plugin->getName();
try {
$pluginName = $plugin->getName();
} catch (Throwable $e) {
$pluginClass = $plugin::class;
error_log("Plugin {$pluginClass} getName() call has failed\n{$e}");
return false;
}
$plugins = & static::getPlugins();

// If the plugin is already loaded or failed/refused to register
if (isset($plugins[$category][$pluginName]) || !$plugin->register($category, $path, $mainContextId)) {
try {
if (isset($plugins[$category][$pluginName]) || !$plugin->register($category, $path, $mainContextId)) {
return false;
}
} catch (Throwable $e) {
$pluginClass = $plugin::class;
error_log("Plugin {$pluginClass} failed to be registered\n{$e}");
return false;
}

Expand Down Expand Up @@ -111,8 +124,8 @@ public static function loadCategory(string $category, bool $enabledOnly = false,
static $cache;
$key = implode("\0", func_get_args());
$plugins = $cache[$key] ??= $enabledOnly && Application::isInstalled()
? static::_loadFromDatabase($category, $mainContextId)
: static::_loadFromDisk($category);
? static::loadFromDatabase($category, $mainContextId)
: static::loadFromDisk($category);

// Fire a hook prior to registering plugins for a category
// n.b.: this should not be used from a PKPPlugin::register() call to "jump categories"
Expand All @@ -130,7 +143,7 @@ public static function loadCategory(string $category, bool $enabledOnly = false,
Hook::call("PluginRegistry::categoryLoaded::{$category}", [&$plugins]);

// Sort the plugins by priority before returning.
uasort($plugins, fn (Plugin $a, Plugin $b) => $a->getSeq() - $b->getSeq());
uasort($plugins, fn (Plugin $a, Plugin $b) => static::getPluginSeq($a) - static::getPluginSeq($b));

return $plugins;
}
Expand All @@ -148,7 +161,7 @@ public static function loadCategory(string $category, bool $enabledOnly = false,
*/
public static function loadPlugin(string $category, string $pluginName, ?int $mainContextId = null): ?Plugin
{
if ($plugin = static::_instantiatePlugin($category, $pluginName)) {
if ($plugin = static::instantiatePlugin($category, $pluginName)) {
static::register($category, $plugin, self::PLUGINS_PREFIX . "{$category}/{$pluginName}", $mainContextId);
}
return $plugin;
Expand Down Expand Up @@ -184,17 +197,22 @@ public static function loadAllPlugins(bool $enabledOnly = false): array
/**
* Instantiate a plugin.
*/
private static function _instantiatePlugin(string $category, string $pluginName, ?string $classToCheck = null): ?Plugin
private static function instantiatePlugin(string $category, string $pluginName, ?string $classToCheck = null): ?Plugin
{
if (!preg_match('/^[a-z0-9]+$/i', $pluginName)) {
throw new Exception("Invalid product name \"{$pluginName}\"");
}

// First, try a namespaced class name matching the installation directory.
$pluginClassName = "\\APP\\plugins\\{$category}\\{$pluginName}\\" . ucfirst($pluginName) . 'Plugin';
$plugin = class_exists($pluginClassName)
? new $pluginClassName()
: static::_deprecatedInstantiatePlugin($category, $pluginName);
try {
$plugin = class_exists($pluginClassName)
? new $pluginClassName()
: static::deprecatedInstantiatePlugin($category, $pluginName);
} catch (Throwable $e) {
error_log("Instantiation of the plugin {$category}/{$pluginName} has failed\n{$e}");
return null;
}

$classToCheck = $classToCheck ?: Plugin::class;
$isObject = is_object($plugin);
Expand All @@ -204,7 +222,7 @@ private static function _instantiatePlugin(string $category, string $pluginName,
}
if ($plugin !== null && !($plugin instanceof $classToCheck)) {
$type = $isObject ? $plugin::class : gettype($plugin);
error_log(new Exception("Plugin {$pluginName} expected to inherit from {$classToCheck}, actual type {$type}"));
error_log(new Exception("Plugin {$category}/{$pluginName} expected to inherit from {$classToCheck}, actual type {$type}"));
return null;
}
return $plugin;
Expand All @@ -213,15 +231,15 @@ private static function _instantiatePlugin(string $category, string $pluginName,
/**
* Attempts to retrieve plugins from the database.
*/
private static function _loadFromDatabase(string $category, ?int $mainContextId = null): array
private static function loadFromDatabase(string $category, ?int $mainContextId = null): array
{
$plugins = [];
$categoryDir = static::PLUGINS_PREFIX . $category;
$products = Application::get()->getEnabledProducts("plugins.{$category}", $mainContextId);
foreach ($products as $product) {
$name = $product->getProduct();
if ($plugin = static::_instantiatePlugin($category, $name, $product->getProductClassname())) {
$plugins[$plugin->getSeq()]["{$categoryDir}/{$name}"] = $plugin;
if ($plugin = static::instantiatePlugin($category, $name, $product->getProductClassname())) {
$plugins[static::getPluginSeq($plugin)]["{$categoryDir}/{$name}"] = $plugin;
}
}
return $plugins;
Expand All @@ -230,7 +248,7 @@ private static function _loadFromDatabase(string $category, ?int $mainContextId
/**
* Get all plug-ins from disk without querying the database, used during installation.
*/
private static function _loadFromDisk(string $category): array
private static function loadFromDisk(string $category): array
{
$categoryDir = static::PLUGINS_PREFIX . $category;
if (!is_dir($categoryDir)) {
Expand All @@ -242,8 +260,8 @@ private static function _loadFromDisk(string $category): array
continue;
}
$pluginName = $path->getFilename();
if ($plugin = static::_instantiatePlugin($category, $pluginName)) {
$plugins[$plugin->getSeq()]["{$categoryDir}/{$pluginName}"] = $plugin;
if ($plugin = static::instantiatePlugin($category, $pluginName)) {
$plugins[static::getPluginSeq($plugin)]["{$categoryDir}/{$pluginName}"] = $plugin;
}
}
return $plugins;
Expand All @@ -254,7 +272,7 @@ private static function _loadFromDisk(string $category): array
*
* @deprecated 3.4.0 Old way to instantiate a plugin
*/
private static function _deprecatedInstantiatePlugin(string $category, string $pluginName): ?Plugin
private static function deprecatedInstantiatePlugin(string $category, string $pluginName): ?Plugin
{
$pluginPath = static::PLUGINS_PREFIX . "{$category}/{$pluginName}";
// Try the plug-in wrapper for backwards compatibility.
Expand All @@ -272,6 +290,20 @@ private static function _deprecatedInstantiatePlugin(string $category, string $p

return null;
}

/**
* Retrieves the plugin's getSeq() or 0 on failure
*/
private static function getPluginSeq(Plugin $plugin): int
{
try {
return $plugin->getSeq();
} catch (Throwable $e) {
$pluginClass = $plugin::class;
error_log("Plugin {$pluginClass} getSeq() call has failed\n{$e}");
return 0;
}
}
}

if (!PKP_STRICT_MODE) {
Expand Down
Loading

0 comments on commit 1b31467

Please sign in to comment.