Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 29121: Catch errors in ->install and ->upgrade calls on plugins

Browse source 
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>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Tomás Cohen Arazi 2021-09-27 08:24:53 -03:00 committed by Jonathan Druart
parent edc8f49335
commit 375a9197b1
2 changed files with 55 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
Koha/Plugins.pm
View file

@ -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();

33
Koha/Plugins/Base.pm
View file

@ -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 });