Skip to content

Simplify OpenVPN and virtual plugin auto-install checks - #3454

Merged
Salvialf merged 2 commits into
developfrom
refactor/plugin-autoinstall-checks
Aug 20, 2026
Merged

Simplify OpenVPN and virtual plugin auto-install checks#3454
Salvialf merged 2 commits into
developfrom
refactor/plugin-autoinstall-checks

Conversation

@Salvialf

@Salvialf Salvialf commented Aug 2, 2026

Copy link
Copy Markdown
Contributor
  • network::dns_create() (OpenVPN) and jeeObject::createSummaryToVirtual() (virtual) both had duplicated install-retry logic between a try block and its catch, with a dead if (!is_object($plugin)) branch that could never trigger since plugin::byId() never returns a non-object without throwing. Both now use plugin::isInstalled() to decide whether an install is needed, removing the duplication and the unreachable branch.
  • As a side effect, this also restores reachability of the intended error message when the second plugin::byId() attempt fails after a failed install (previously, an uncaught exception from that second call bypassed the friendly "plugin must be installed" message).
  • createSummaryToVirtual()'s sleep(2) after doUpdate() is commented out rather than removed: it dates back to a 2016 fix with no documented root cause, doUpdate() is fully synchronous today, and unlike OpenVPN this flow never calls dependancy_install(). Kept as a comment pending real-world confirmation it's safe to drop entirely.

@Salvialf Salvialf added the changelog-fix Use to generate release notes / changelog To be apply on PR label Aug 2, 2026
@Salvialf
Salvialf merged commit c24e8b4 into develop Aug 20, 2026
6 checks passed
@Salvialf
Salvialf deleted the refactor/plugin-autoinstall-checks branch August 20, 2026 11:27
@Mips2648 Mips2648 added this to the 5.0 milestone Sep 1, 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.

3 participants