@@ -, +, @@ to network problems #.....your implementation..... # Throw an exception in case of a connection error # $connError can be for example: ($curl->{'retcode'} == 6) # cURL code 6: CURLE_COULDNT_RESOLVE_HOST if ($connError){ Koha::Exceptions::ConnectionFailed->throw(error => "Connection failed"); } to your SMS::Send driver send_sms. "Connection failed. Attempting to resend." --- C4/Letters.pm | 42 ++++++++++++++++++++++++++++++++++------ C4/SMS.pm | 21 ++++++++++++-------- Koha/Exceptions.pm | 4 ++++ t/db_dependent/Letters.t | 32 +++++++++++++++++++++++++++++- 4 files changed, 84 insertions(+), 15 deletions(-) --- a/C4/Letters.pm +++ a/C4/Letters.pm @@ -26,6 +26,8 @@ use Encode; use Carp; use Template; use Module::Load::Conditional qw(can_load); +use Scalar::Util qw ( blessed ); +use Try::Tiny; use C4::Members; use C4::Members::Attributes qw(GetBorrowerAttributes); @@ -1392,12 +1394,40 @@ sub _send_message_by_sms { return; } - my $success = C4::SMS->send_sms( { destination => $patron->smsalertnumber, - message => $message->{'content'}, - } ); - _set_message_status( { message_id => $message->{'message_id'}, - status => ($success ? 'sent' : 'failed'), - delivery_note => ($success ? '' : 'No notes from SMS driver') } ); + my $success; + try { + $success = C4::SMS->send_sms( + { destination => $patron->smsalertnumber, + message => $message->{'content'}, + } ); + _set_message_status( { message_id => $message->{'message_id'}, + status => ($success ? 'sent' : 'failed'), + delivery_note => ($success ? '' : 'No notes from SMS driver') } ); + + } catch { + if (blessed($_)){ + if ($_->isa('Koha::Exceptions::ConnectionFailed')){ + # Keep the message in pending status but + # add a delivery note explaining what happened + _set_message_status ( { message_id => $message->{'message_id'}, + status => 'pending', + delivery_note => 'Connection failed. Attempting to resend.' } ); + } + else { + # failsafe: if we catch and unknown exception, set message status to failed + _set_message_status( { message_id => $message->{'message_id'}, + status => 'failed', + delivery_note => ref($_) } ); + $_->rethrow(); + } + } + else { + _set_message_status( { message_id => $message->{'message_id'}, + status => 'failed', + delivery_note => 'Unknown exception.' } ); + die $_; + } + }; return $success; } --- a/C4/SMS.pm +++ a/C4/SMS.pm @@ -56,6 +56,8 @@ use warnings; use C4::Context; use File::Spec; +use Try::Tiny; +use Scalar::Util qw ( blessed ); =head1 METHODS @@ -107,7 +109,9 @@ sub send_sms { %args = map { q{_} . $_ => $conf->{$_} } keys %$conf; } - eval { + #We might die because SMS::Send $driver is not defined or the sms-number has a bad format + #Catch those errors and fail the sms-sending gracefully. + try { # Create a sender $sender = SMS::Send->new( $driver, @@ -121,15 +125,16 @@ sub send_sms { to => $params->{destination}, text => $params->{message}, ); + return $sent; + } catch { + if (blessed($_) && $_->can('rethrow')) { + $_->rethrow(); + } + else { + die $_; + } }; - #We might die because SMS::Send $driver is not defined or the sms-number has a bad format - #Catch those errors and fail the sms-sending gracefully. - if ($@) { - warn $@; - return; - } - # warn 'failure' unless $sent; return $sent; } --- a/Koha/Exceptions.pm +++ a/Koha/Exceptions.pm @@ -22,6 +22,10 @@ use Exception::Class ( isa => 'Koha::Exceptions::Exception', description => 'The default value cannot be deleted' }, + 'Koha::Exceptions::ConnectionFailed' => { + isa => 'Koha::Exceptions::Exception', + description => 'Connecting to host failed', + }, 'Koha::Exceptions::MissingParameter' => { isa => 'Koha::Exceptions::Exception', description => 'A required parameter is missing' --- a/t/db_dependent/Letters.t +++ a/t/db_dependent/Letters.t @@ -18,11 +18,12 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 69; +use Test::More tests => 70; use Test::MockModule; use Test::Warn; use MARC::Record; +use Module::Load::Conditional qw/check_install/; my %mail; my $module = new Test::MockModule('Mail::Sendmail'); @@ -160,6 +161,35 @@ is( $resent, undef, 'ResendMessage should return undef if not message_id given' is($messages->[0]->{delivery_note}, 'Missing SMS number', 'Delivery note for no smsalertnumber correctly set'); +subtest 'Test SMS::Send driver connectivity exception' => sub { + if ( check_install( module => 'SMS::Send::Test') ) { + plan tests => 3; + } else { + plan skip_all => 'SMS::Send not installed'; + } + + + my $sms_send_module = new Test::MockModule('SMS::Send::Test'); + $sms_send_module->mock( + 'send_sms', + sub { + warn "Fake SMS driver"; + Koha::Exceptions::ConnectionFailed->throw(error => "Connection failed"); + } + ); + t::lib::Mocks::mock_preference('SMSSendDriver', 'Test'); + Koha::Patrons->find($borrowernumber)->set({ smsalertnumber => "+1234567890" })->store; + warning_is { $messages_processed = C4::Letters::SendQueuedMessages(); } + "Fake SMS driver", + "SMS sent using the mocked SMS::Send driver subroutine send_sms"; + $messages = C4::Letters::GetQueuedMessages(); + is( $messages->[0]->{status}, 'pending', + 'Message is still pending after SendQueuedMessages() because of network failure (bug 14791)' ); + is( $messages->[0]->{delivery_note}, 'Connection failed. Attempting to resend.', + 'Message has correct delivery note about resending' ); +}; + + # GetLetters my $letters = C4::Letters::GetLetters(); --