From 6acae7bbe4386cae678fe2fd25c58d9d16738ec6 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
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 <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 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.20.1