[Bug 26922] New: SendAlerts does not correctly handle error on sending emails
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Bug ID: 26922 Summary: SendAlerts does not correctly handle error on sending emails Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: System Administration Assignee: tomascohen@gmail.com Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Depends on: 22343 When an email is sent from C4::Letters::SendAlerts (for instance emailing a acquisition basket), the UI displays a "all good" message even if the email has not been sent properly. To recreate: 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 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. The problem is coming from the return inside the catch (which returns from the catch, not the subroutine as expected). There are 2 problematic occurrences in SendAlerts: 360 try { 361 $mail->send_or_die({ transport => $library->smtp_server->transport }); 362 } 363 catch { 364 carp "$_"; 365 return { error => "$_" }; 366 }; 512 try { 513 $mail->send_or_die({ transport => $library->smtp_server->transport }); 514 } 515 catch { 516 carp "$_"; 517 return { error => "$_" }; 518 }; => SendAlerts will return 1 (from l.577) anyway. And 1 occurrence in _send_message_by_email that should be corrected to avoid confusion. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22343 [Bug 22343] Add configuration options for SMTP servers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_11_target -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113736&action=edit Bug 26922: Correctly handle email sending error from SendAlerts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- There is something weird going on. With this patch I get a stacktrace of the "unable to establish SMTP connection to localhost port 25" error. Which is okish. The problem is that I've tried to turn off dev_install and see if the error was nicer, and... there is no error now. (Worst, I put a warn in mainpage.pl and I did not find in which log file it landed) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113736|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 113828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113828&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I'll try to provide some tests for this later. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #5 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
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.
Can't reproduce, in the UI I have «Order e-mail was sent to the vendor.» The logs contain the error. env: koha-testing-docker code: master without patch applied In the vendor setup I filled an email and ticked «Contact when ordering?» -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #5)
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.
Can't reproduce, in the UI I have «Order e-mail was sent to the vendor.» The logs contain the error.
env: koha-testing-docker code: master without patch applied
In the vendor setup I filled an email and ticked «Contact when ordering?»
I clicked on the 'E-mail order' button. https://snipboard.io/6Yn8W1.jpg -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #7 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Same and that's how I got (and still get after retrying) a success instead of a failure. When I apply the patch and retry, I get.
ERROR! - unable to establish SMTP connection to localhost port 25
Which is good! Even if I don't get the same level of wrong behavior without the patch, the test seems to be a success right? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Yes, we are basically providing a better error message. The original issue was because of dev_install=0, which ended up in a variety of behaviors depending on the age of you docker image. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113828|0 |1 is obsolete| | --- Comment #9 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 113849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113849&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113849|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113870&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This works well and passes the QA scripts. Were you still going to add Unit tests to cover the change of return Tomas? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 113884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113884&action=edit Bug 26922: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113870|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113894&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113884|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113895&action=edit Bug 26922: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Works well, covered by tests.. Thanks Tomas Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_11_target | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Bug 26922 depends on bug 22343, which changed state. Bug 22343 Summary: Add configuration options for SMTP servers https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22343 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26922 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #17 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Missing dependencies, not backported to 20.05 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org