[Bug 28803] New: process_message_queue.pl dies if any messsages in the message queue contain an invalid to_address
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Bug ID: 28803 Summary: process_message_queue.pl dies if any messsages in the message queue contain an invalid to_address Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: hagud@orex.es, koha-bugs@lists.koha-community.org, testopia@bugs.koha-community.org We try hard to catch bad email addresses on entry, but we don't always succeed for cases like patron imports. However, with the introduction of exceptions in Koha::Email->create on invalid addresses we now die upon meeting such a case in the database. We should instead wrap this in a try/catch block and report the failure in the database and then move on to the next message in the queue instead of exploding. -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|koha-bugs@lists.koha-commun | |ity.org, | |testopia@bugs.koha-communit | |y.org | -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123469 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123469&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123470 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123470&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL' and the exception message being recorded in the delivery_note. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28813 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28813 [Bug 28813] Delivery notes feature was pushed prior to it's completion -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |BLOCKED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Whilst writing tests for this, I found some issues with bug 14723 and have hence added a dependency on bug 28813 which I will use to fix those issues and add more comprehensive tests. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123469|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123475 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123475&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123470|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123476 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123476&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Rebased on top of bug 28813. I'd have liked to have enhanced the failure_code handling so we could continue to pass in which email parameter failed validation as the first version of this patch did but I felt that was out of scope here. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Tomás Cohen Arazi <tomascohen@gmail.com> 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=28803 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123475|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=28803 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123476|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=28803 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 124000 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124000&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. 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=28803 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 124001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124001&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. 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=28803 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This is the right short-term solution for this issue. A more global solution should be implemented on bug 28870. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Status|Signed Off |BLOCKED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + [% ELSIF ( QUEUED_MESSAGE.failure_code == "INVALID_EMAIL" ) %]Invalid email address found [% borrowernumber | html %] This is probably the most obvious reason, but actually we only know that Koha::Email->create crashed (or Email::Stuffer->new). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124000|0 |1 is obsolete| | --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124131&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124001|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124132&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- - # FIXME must be 'failed' - is( $message_1->{status}, 'pending', 'Invalid KohaAdminEmailAddress => status pending' ); + is( $message_1->{status}, 'failed', 'Invalid KohaAdminEmailAddress => status failed' ); That's very pleasant to read :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #13)
- # FIXME must be 'failed' - is( $message_1->{status}, 'pending', 'Invalid KohaAdminEmailAddress => status pending' ); + is( $message_1->{status}, 'failed', 'Invalid KohaAdminEmailAddress => status failed' );
That's very pleasant to read :)
Haha, it made me smile too :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This is tight to bug 28870. I think we should push both altogether. However there is one thing that seems wrong to me: + my $email = try { + Koha::Email->create( [SKIP] + } + catch { + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'failed', + failure_code => 'INVALID_EMAIL' + } + ); + return 0; + }; Koha::Email->create is raising different exception, and Email::Stuffer certainly raised its own. Shouldn't we actually base this on top of bug 28870 and simply call (the newly added) Koha::Email->is_valid to check if the email if valid? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28870 Status|Passed QA |Failed QA --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- IMO we should base this on top of bug 28870 and provide a separate simple patch for backporting. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28870 [Bug 28870] Cart shipping fails because of Non-ASCII characters in display-name of reply-to address -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- The easy way here is actually to keep the existent patches and replace "INVALID_EMAIL" by "UNKNOWN_ERROR", and we follow-up on a separate bug reports with the different exceptions/errors possible. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #17)
The easy way here is actually to keep the existent patches and replace "INVALID_EMAIL" by "UNKNOWN_ERROR", and we follow-up on a separate bug reports with the different exceptions/errors possible.
What do you think?
Seems reasonable to me.. I think.. Must admit though, my brain has moved on a bit since I wrote this.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Hans Pålsson <hans.palsson@hkr.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hans.palsson@hkr.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 124746 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124746&action=edit Bug 28803: Replace INVALID_EMAIL with UNKNOWN_ERROR There can be other exceptions/errors raised by Koha::Email->create. On a follow-up bug report we should deal with the different possible errors to handle them properly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Everything ok with this approach then? Please keep moving! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Sorry, not sure what more needs doing here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #22 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #21)
Sorry, not sure what more needs doing here?
Confirmation from you and Tomas that the last patch is what we want. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Well.. all the exceptions thrown by Koha::Email->create are around invalid email addresses so I'm not entirely comfortable with 'Unknown error'.. that kinda feels like "Give up now" to me whereas the actual fix is fairly simple. Problem is.. we don't, yet, pass the 'fields' param in the BadParameter exceptions that are thrown and I'm not sure we've got a field in the messages table to hold that data anyway to pass it to the template. So.. all in all.. I dunno. Personally, we have the patch in production now on all our servers without your follow-up.. and find it helpful. I also don't really think this needs to require either of the depends. Yes, they help to reduce the times it'll get thrown, but this patch eliminates a vital issue whereby sending ANY massages in the queue can be thwarted by one bad email address server wide. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124829&action=edit Bug 28803: (follow-up) Error details improvement This patch updates the exceptions thrown by Koha::Email to include the parameter that failed email validation and then updates the failure code to include this parameter and finally display this field in the template. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- My final patch is an alternative to Jonathans.. it tries to improve the feedback given to the end user by embedding the parameter that failed validation into the error code and then adding this to the template output. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124746|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=28803 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Is the last patch failing to apply for anyone else? Applying: Bug 28803: (follow-up) Error details improvement .git/rebase-apply/patch:71: trailing whitespace. Koha::Exceptions::BadParameter->throw({ parameter => $address, error => error: sha1 information is lacking or useless (C4/Letters.pm). error: could not build fake ancestor Patch failed at 0001 Bug 28803: (follow-up) Error details improvement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124131|0 |1 is obsolete| | --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124878 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124878&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124132|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124879&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124829|0 |1 is obsolete| | --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124880 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124880&action=edit Bug 28803: (follow-up) Error details improvement This patch updates the exceptions thrown by Koha::Email to include the parameter that failed email validation and then updates the failure code to include this parameter and finally display this field in the template. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Fresh patches attached... should work now Kyle, sorry about that... sha1 issues are the bane. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #31 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #30)
Fresh patches attached... should work now Kyle, sorry about that... sha1 issues are the bane.
I looks like your patches conflict when applied on top of 28870? Even applied directly to master, I'm getting unit test failures: t/db_dependent/Letters.t .. 51/83 # Failed test 'Failure code set correctly for invalid email parameter' # at t/db_dependent/Letters.t line 959. # got: 'INVALID_EMAIL:HASH(0x55de0d87b468)->parameter' # expected: 'INVALID_EMAIL' # Looks like you failed 1 test of 7. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124878|0 |1 is obsolete| | --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124886&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124879|0 |1 is obsolete| | --- Comment #33 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124887&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124880|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124888 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124888&action=edit Bug 28803: (follow-up) Error details improvement This patch updates the exceptions thrown by Koha::Email to include the parameter that failed email validation and then updates the failure code to include this parameter and finally display this field in the template. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Rebased for bug 28870.. I really wish the dependencies hadn't been jumbled up so much.. this was a trivial and critical fix for older Koha's and now it's got a chain of bugs to apply. Oh well.. this is how things go sometimes I suppose. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124886|0 |1 is obsolete| | Attachment #124887|0 |1 is obsolete| | Attachment #124888|0 |1 is obsolete| | --- Comment #36 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 124900 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124900&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #37 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 124901 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124901&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #38 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 124902 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124902&action=edit Bug 28803: (follow-up) Error details improvement This patch updates the exceptions thrown by Koha::Email to include the parameter that failed email validation and then updates the failure code to include this parameter and finally display this field in the template. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Isn't UNKNOWN_ERROR missing in notices.tt? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125114&action=edit Bug 28803: (QA folow-up) Reinstate UNKNOWN_ERROR in templates I can't see how this will ever get thrown, but as it's there we should have the template handling included. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #41 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125115 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125115&action=edit Bug 28803: (QA follow-up) Reinstate UNKNOWN_ERROR in templates I can't see how this will ever get thrown, but as it's there we should have the template handling included. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125114|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=28803 --- Comment #42 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Done... isn't this already PQA.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #42)
Done... isn't this already PQA..
There is only 1 SO-by line on the patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joonas.kylmala@iki.fi --- Comment #44 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Martin Renvoize from comment #41)
Created attachment 125115 [details] [review] Bug 28803: (QA follow-up) Reinstate UNKNOWN_ERROR in templates
I can't see how this will ever get thrown, but as it's there we should have the template handling included.
If the SMTP server is unavailable it at least comes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124900|0 |1 is obsolete| | Attachment #124901|0 |1 is obsolete| | Attachment #124902|0 |1 is obsolete| | Attachment #125115|0 |1 is obsolete| | --- Comment #45 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125343&action=edit Bug 28803: Add unit tests This patch updates Letters.t to confirm that invalid email addresses in the message_queue should not throw an exception when sending mail but instead set the status to failed and pass error details to the end user. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #46 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125344 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125344&action=edit Bug 28803: Add invalid email handling in _send_message_by_email This patch adds a try/catch block around the call to Koha::Email->create to catch and handle invalid emails being passed in the parameters. The message is marked as 'failed' with an error_code of 'INVALID_EMAIL'. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #47 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125345 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125345&action=edit Bug 28803: (follow-up) Error details improvement This patch updates the exceptions thrown by Koha::Email to include the parameter that failed email validation and then updates the failure code to include this parameter and finally display this field in the template. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #48 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125346 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125346&action=edit Bug 28803: (QA follow-up) Reinstate UNKNOWN_ERROR in templates I can't see how this will ever get thrown, but as it's there we should have the template handling included. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #49 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Joonas Kylmälä from comment #44)
(In reply to Martin Renvoize from comment #41)
Created attachment 125115 [details] [review] [review] Bug 28803: (QA follow-up) Reinstate UNKNOWN_ERROR in templates
I can't see how this will ever get thrown, but as it's there we should have the template handling included.
If the SMTP server is unavailable it at least comes.
Really? I didn't think that was the case when calling 'create'.. if it is the case I don't understand why, we don't do anything with the SMTP server until the call to send_or_die. It doesn't cause any harm having this, but I still haven't been able to trigger it myself. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #50 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #49)
(In reply to Joonas Kylmälä from comment #44)
(In reply to Martin Renvoize from comment #41)
Created attachment 125115 [details] [review] [review] [review] Bug 28803: (QA follow-up) Reinstate UNKNOWN_ERROR in templates
I can't see how this will ever get thrown, but as it's there we should have the template handling included.
If the SMTP server is unavailable it at least comes.
Really? I didn't think that was the case when calling 'create'.. if it is the case I don't understand why, we don't do anything with the SMTP server until the call to send_or_die.
It doesn't cause any harm having this, but I still haven't been able to trigger it myself.
I confirm it's needed: https://snipboard.io/10dPzT.jpg kohadev-koha@kohadevbox:/kohadevbox/koha$ perl misc/cronjobs/process_message_queue.pl unable to establish SMTP connection to (localhost) port 25 Trace begun at /usr/share/perl5/Email/Sender/Transport/SMTP.pm line 224 Email::Sender::Transport::SMTP::_throw('Email::Sender::Transport::SMTP=HASH(0x55bd3b7b9bd0)', 'unable to establish SMTP connection to (localhost) port 25') called at /usr/share/perl5/Email/Sender/Transport/SMTP.pm line 174 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.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=28803 --- Comment #51 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.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=28803 Bug 28803 depends on bug 28870, which changed state. Bug 28870 Summary: Cart shipping fails because of Non-ASCII characters in display-name of reply-to address https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28870 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=28803 Bug 28803 depends on bug 28870, which changed state. Bug 28870 Summary: Cart shipping fails because of Non-ASCII characters in display-name of reply-to address https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28870 What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #52 from Kyle M Hall <kyle@bywatersolutions.com> --- Please add patches for 21.05.x if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28996 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |didier.gautheron@biblibre.c | |om --- Comment #53 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 28996 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29330 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29330 [Bug 29330] Koha cannot send emails with attachments using Koha::Email and message_queue table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|29330 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29330 [Bug 29330] Koha cannot send emails with attachments using Koha::Email and message_queue table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #54 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Kyle M Hall from comment #52)
Please add patches for 21.05.x if needed.
Patches apply on 21.05.x I suppose it's thanks to bug 28870 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00 |21.11.00,21.05.05 released in| | Status|Pushed to master |Pushed to stable --- Comment #55 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00,21.05.05 |21.11.00,21.05.05,20.11.12 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to stable |Pushed to oldstable --- Comment #56 from Fridolin Somers <fridolin.somers@biblibre.com> --- I've backported unit test to 20.11.x (just removed failure code part) : 2712dbeca6 Bug 28803: Add unit tests And since Bug 28813 is not in 20.11.x, I applied a simplified version based on Bug 28996 : c8a36a1b95 Bug 28803: (20.11.x) Add invalid email handling in _send_message_by_email -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 --- Comment #57 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29330 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED --- Comment #58 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to 20.05.x which has reached end-of-line and won't be supported anymore. Not worth the risks of a tricky backport for a last release. Which is already late due to a blocking bug. So I won't seek more trouble ^^" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28803 Bug 28803 depends on bug 28870, which changed state. Bug 28870 Summary: Cart shipping fails because of Non-ASCII characters in display-name of reply-to address https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28870 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |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=28803 --- Comment #59 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 24787 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org