From a9e3db201e1863b30ac0e2654d82a15f0f8e44c0 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 19 Nov 2020 13:09:47 -0300 Subject: [PATCH] Bug 26922: Better error handling in SendAlerts This patch makes SendAlerts display a better error message when sending fails. To test: 1. Set KohaAdminEmailAddress to admin@example.org 2. Edit a vendor, set a valid email address 3. Create a new basket, a new order. Send the basket => FAIL: As you did not configure a valid SMTP server, the email is not sent and logs displayed "unable to establish SMTP connection to (localhost) port 25", with the stracktrace. 4. Apply this patch and reload all 5. Repeat 3 => SUCCESS: A simpler message is displayed, the stacktrace remains in the logs 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart --- C4/Letters.pm | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index d3e354f6c6..9383b71a3f 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -294,6 +294,8 @@ sub DelLetter { sub SendAlerts { my ( $type, $externalid, $letter_code ) = @_; my $dbh = C4::Context->dbh; + my $error; + if ( $type eq 'issue' ) { # prepare the letter... @@ -357,13 +359,19 @@ sub SendAlerts { $mail->text_body( $letter->{content} ); } - try { + my $success = try { $mail->send_or_die({ transport => $library->smtp_server->transport }); } catch { + # We expect ref($_) eq 'Email::Sender::Failure' + $error = $_->message; + carp "$_"; - return { error => "$_" }; + return; }; + + return { error => $error } + unless $success; } } elsif ( $type eq 'claimacquisition' or $type eq 'claimissues' or $type eq 'orderacquisition' ) { @@ -511,14 +519,20 @@ sub SendAlerts { $mail->text_body( "" . $letter->{content} ); } - try { + my $success = try { $mail->send_or_die({ transport => $library->smtp_server->transport }); } catch { + # We expect ref($_) eq 'Email::Sender::Failure' + $error = $_->message; + carp "$_"; - return { error => "$_" }; + return; }; + return { error => $error } + unless $success; + logaction( "ACQUISITION", $action, @@ -547,7 +561,8 @@ sub SendAlerts { want_librarian => 1, ) or return; return { error => "no_email" } unless $externalid->{'emailaddr'}; - try { + + my $success = try { # FIXME: This 'default' behaviour should be moved to Koha::Email my $mail = Koha::Email->create( @@ -570,9 +585,15 @@ sub SendAlerts { $mail->send_or_die({ transport => $library->smtp_server->transport }); } catch { + # We expect ref($_) eq 'Email::Sender::Failure' + $error = $_->message; + carp "$_"; - return { error => "$_" }; + return; }; + + return { error => $error } + unless $success; } # If we come here, return an OK status -- 2.39.5