From 2fe9f5c3304a986e544e5ebe1f7e39e28d752126 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