Skip to content

Restore unreachable error handling when plugin::byId can't find a plugin - #3455

Open
Salvialf wants to merge 2 commits into
developfrom
fix/plugin-byid-errors
Open

Restore unreachable error handling when plugin::byId can't find a plugin#3455
Salvialf wants to merge 2 commits into
developfrom
fix/plugin-byid-errors

Conversation

@Salvialf

@Salvialf Salvialf commented Aug 2, 2026

Copy link
Copy Markdown
Contributor
  • plugin::byId() is the only byId() method in the entire core/class/ directory that throws an exception instead of returning a falsy value when nothing is found (verified against all 27 other byId() implementations, which are plain DB lookups). Several call sites across core were written assuming the more common convention, guarding the result with if (!is_object($plugin)). Since byId() throws before ever returning, these guards were dead code, and the intended fallback behavior (a friendly error message, a graceful "not found" response, skipping one entry in a loop) was never reachable.
  • Each affected call now wraps plugin::byId() in a try/catch (or, in utils.inc.php's loop over multiple plugins, a plugin::isInstalled() pre-check), restoring the originally intended fallback behavior instead of letting a generic exception propagate to the nearest outer handler.
  • In core/ajax/plugin.ajax.php, also removed two $plugin_id = init('id') assignments left unused in getDeamonInfo and deamonStart, found while touching these same blocks.
  • jsonrpc::makeError() and jsonrpc::makeSuccess() are now documented with @return never, mirroring ajax::success()/ajax::error()'s existing annotations. Without it, PHPStan flagged $plugin as possibly undefined after the try/catch blocks in core/api/jeeApi.php, since it couldn't infer that the catch branch's call to makeSuccess() always ends execution.

@Salvialf Salvialf added the changelog-fix Use to generate release notes / changelog To be apply on PR label Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-fix Use to generate release notes / changelog To be apply on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant