Bugzilla – Attachment 125479 Details for
Bug 29121
Plugins with broken ->install prevent access to the plugins list
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29121: Catch errors in ->install and ->upgrade calls on plugins
Bug-29121-Catch-errors-in--install-and--upgrade-ca.patch (text/plain), 6.31 KB, created by
Martin Renvoize (ashimema)
on 2021-09-30 09:59:31 UTC
(
hide
)
Description:
Bug 29121: Catch errors in ->install and ->upgrade calls on plugins
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-09-30 09:59:31 UTC
Size:
6.31 KB
patch
obsolete
>From 99b55a5b0bb4a153888043bbfd29667f991d650b Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 27 Sep 2021 08:24:53 -0300 >Subject: [PATCH] Bug 29121: Catch errors in ->install and ->upgrade calls on > plugins > >This patch adds a try/catch block when instantiating plugins. Calling >->new on a plugin eventually triggers a call to ->install (this has >always been like this since bug 7804). If the ->install method is >somehow borked, then the process dies. We need to prevent that, and >report back some error took place. That's what this patch does. > >The same happens to the ->upgrade. > >To test: >1. Install any plugin you like >2. Restart plack (just in case) >=> SUCCESS: All good >3. Manually change its install method to: > >sub install { > die "plugin, die!"; >} > >4. Run: > $ koha-mysql kohadev > > DELETE FROM plugin_data; >(to make sure there's no __INSTALLED__ entry, do on a safe to delete DB). >5. Point your browser to the plugins-home.pl page >=> FAIL: Boom >6. Apply up to the regression tests >7. Run: > $ kshell > k$ prove t/db_dependent/Koha/Plugins/Plugins.t \ > t/Koha/Exceptions.t >=> FAIL: Tests fail! >8. Apply this patch >9. Repeat 2 >=> SUCCESS: Tests pass! >10. Run: > $ restart_all >11. Repeat 5 >=> SUCCESS: The page is not broken >12. Sign off :-D > >Note: I used > $ kshell > k$ perl misc/devel/install_plugins.pl >to test as well. > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/Plugins.pm | 39 ++++++++++++++++++++++++++++++++------- > Koha/Plugins/Base.pm | 33 +++++++++++++++++++++++---------- > 2 files changed, 55 insertions(+), 17 deletions(-) > >diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm >index b8f9f86a33..fa4d235287 100644 >--- a/Koha/Plugins.pm >+++ b/Koha/Plugins.pm >@@ -25,10 +25,12 @@ use List::MoreUtils qw( any ); > use Module::Load::Conditional qw( can_load ); > use Module::Load; > use Module::Pluggable search_path => ['Koha::Plugin'], except => qr/::Edifact(|::Line|::Message|::Order|::Segment|::Transport)$/; >- >+use Try::Tiny; > > use C4::Context; > use C4::Output; >+ >+use Koha::Exceptions::Plugin; > use Koha::Plugins::Methods; > > BEGIN { >@@ -120,11 +122,23 @@ sub GetPlugins { > while ( my $plugin_class = $plugin_classes->next ) { > > if ( can_load( modules => { $plugin_class => undef }, nocache => 1 ) ) { >- my $plugin = $plugin_class->new({ >- enable_plugins => $self->{'enable_plugins'} >- # loads even if plugins are disabled >- # FIXME: is this for testing without bothering to mock config? >- }); >+ >+ my $plugin; >+ my $failed_instantiation; >+ >+ try { >+ $plugin = $plugin_class->new({ >+ enable_plugins => $self->{'enable_plugins'} >+ # loads even if plugins are disabled >+ # FIXME: is this for testing without bothering to mock config? >+ }); >+ } >+ catch { >+ warn "$_"; >+ $failed_instantiation = 1; >+ }; >+ >+ next if $failed_instantiation; > > next unless $plugin->is_enabled or > defined($params->{all}) && $params->{all}; >@@ -169,7 +183,18 @@ sub InstallPlugins { > if ( can_load( modules => { $plugin_class => undef }, nocache => 1 ) ) { > next unless $plugin_class->isa('Koha::Plugins::Base'); > >- my $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} }); >+ my $plugin; >+ my $failed_instantiation; >+ >+ try { >+ $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} }); >+ } >+ catch { >+ warn "$_"; >+ $failed_instantiation = 1; >+ }; >+ >+ next if $failed_instantiation; > > Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete(); > >diff --git a/Koha/Plugins/Base.pm b/Koha/Plugins/Base.pm >index 10c66d7486..92ce7714bd 100644 >--- a/Koha/Plugins/Base.pm >+++ b/Koha/Plugins/Base.pm >@@ -21,12 +21,15 @@ use Modern::Perl; > > use Cwd qw( abs_path ); > use List::Util qw( max ); >+use Try::Tiny; > > use base qw{Module::Bundled::Files}; > > use C4::Context; > use C4::Output qw( output_with_http_headers ); > >+use Koha::Exceptions::Plugin; >+ > =head1 NAME > > Koha::Plugins::Base - Base Module for plugins >@@ -48,21 +51,31 @@ sub new { > > ## Run the installation method if it exists and hasn't been run before > if ( $self->can('install') && !$self->retrieve_data('__INSTALLED__') ) { >- if ( $self->install() ) { >- $self->store_data( { '__INSTALLED__' => 1, '__ENABLED__' => 1 } ); >- if ( my $version = $plugin_version ) { >- $self->store_data({ '__INSTALLED_VERSION__' => $version }); >+ try { >+ if ( $self->install() ) { >+ $self->store_data( { '__INSTALLED__' => 1, '__ENABLED__' => 1 } ); >+ if ( my $version = $plugin_version ) { >+ $self->store_data({ '__INSTALLED_VERSION__' => $version }); >+ } >+ } else { >+ warn "Plugin $class failed during installation!"; > } >- } else { >- warn "Plugin $class failed during installation!"; > } >+ catch { >+ Koha::Exceptions::Plugin::InstallDied->throw( plugin_class => $class ); >+ }; > } elsif ( $self->can('upgrade') ) { > if ( _version_compare( $plugin_version, $database_version ) == 1 ) { >- if ( $self->upgrade() ) { >- $self->store_data({ '__INSTALLED_VERSION__' => $plugin_version }); >- } else { >- warn "Plugin $class failed during upgrade!"; >+ try { >+ if ( $self->upgrade() ) { >+ $self->store_data({ '__INSTALLED_VERSION__' => $plugin_version }); >+ } else { >+ warn "Plugin $class failed during upgrade!"; >+ } > } >+ catch { >+ Koha::Exceptions::Plugin::UpgradeDied->throw( plugin_class => $class ); >+ }; > } > } elsif ( $plugin_version ne $database_version ) { > $self->store_data({ '__INSTALLED_VERSION__' => $plugin_version }); >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29121
:
125352
|
125353
|
125354
|
125357
|
125358
|
125359
|
125360
|
125361
|
125477
|
125478
| 125479 |
125480