Bug 3186 - Invalid or uninstalled SMSSendDriver (or bad number format) causes process_message_queue to fail
Summary: Invalid or uninstalled SMSSendDriver (or bad number format) causes process_me...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: PC All
: P5 - low normal
Assignee: Bugs List
QA Contact:
URL:
Keywords:
: 14611 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-05-04 10:05 UTC by Chris Cormack
Modified: 2023-04-08 15:21 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
pätch (1.86 KB, patch)
2014-08-27 13:24 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
pätch (1.80 KB, patch)
2014-08-27 13:53 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 3186 - invalid or uninstalled SMSSendDriver (or bad number format) causes process_message_queue to fail (1.85 KB, patch)
2014-08-27 23:25 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 3186 [QA Followup] - Don't return undef, just return (711 bytes, patch)
2014-08-29 14:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 3186 - invalid or uninstalled SMSSendDriver (or bad number format) causes process_message_queue to fail (1.92 KB, patch)
2014-08-29 14:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 3186 [QA Followup] - Don't return undef, just return (711 bytes, patch)
2014-08-29 14:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:07:27 UTC


---- Reported by rch@liblime.com 2009-05-04 22:05:18 ----

If a user enters a driver into the SMSSendDriver system preference that is not installed on the system, the message queue cron job will exit abnormally when attempting to process sms messages, preventing processing of other messages in the queue.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:07 UTC  ---

This bug was previously known as _bug_ 3186 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3186

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was rch@liblime.com.

Comment 1 Olli-Antti Kivilahti 2014-08-13 10:08:59 UTC
This also happens when the smsalertnumber is of the following format:

040-12345678(mother)

This is caused by the /usr/share/perl5/SMS/Send.pm parent object for sms sending.
It tries to validate the number and Croacks the process dead when (mother) is encountered.

Luckily verbose mode outputs goog logging info :)


Recommending to add a presentation layer validator to OPAC and SC to make sure no mothers are included.
Comment 2 Olli-Antti Kivilahti 2014-08-27 13:24:10 UTC Comment hidden (obsolete)
Comment 3 Olli-Antti Kivilahti 2014-08-27 13:53:31 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2014-08-27 23:25:24 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall (khall) 2014-08-29 14:39:17 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall (khall) 2014-08-29 14:39:33 UTC
Created attachment 31291 [details] [review]
[PASSED QA] Bug 3186 - invalid or uninstalled SMSSendDriver (or bad number format) causes process_message_queue to fail

This patch wraps the SMS::Send Driver initialization and sending process into an eval block
  and gracefully causes the sending to fail, instead of crashing the process and preventing
  emails and good sms' from being sent.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall (khall) 2014-08-29 14:39:43 UTC
Created attachment 31292 [details] [review]
[PASSED QA] Bug 3186 [QA Followup] - Don't return undef, just return

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Tomás Cohen Arazi (tcohen) 2014-10-30 04:28:22 UTC
Patches pushed to master.

Thanks Olli!
Comment 9 Olli-Antti Kivilahti 2014-10-30 06:10:25 UTC
Must be my lucky week!
Comment 10 Jonathan Druart 2015-01-29 10:09:16 UTC
Mason, Fridolin,
IMO it makes sense to backport this one.
Comment 11 Mason James 2015-02-25 11:50:35 UTC
(In reply to Jonathan Druart from comment #10)
> Mason, Fridolin,
> IMO it makes sense to backport this one.

cheers, i'll take a look
Comment 12 Mason James 2015-02-25 11:57:45 UTC
(In reply to Mason James from comment #11)
> (In reply to Jonathan Druart from comment #10)
> > Mason, Fridolin,
> > IMO it makes sense to backport this one.
> 
> cheers, i'll take a look

Pushed to 3.16.x branch. Patch will be in included in 3.16.8 release
Comment 13 Fred P 2015-07-28 20:18:42 UTC
I feel like there are two bugs here - the missing SMS driver, which is patched and the bad number format, which is not. Could we split that out as a separate bug? It would be pretty easy to regex the number before we drop it into the database. I understand that phone numbers are often stored as text, because of hyphens and parentheses, but if stray symbols break the whole overdue notifications process, we might want to strip phone numbers down to the digits.
Comment 14 Jonathan Druart 2015-08-19 15:17:51 UTC
*** Bug 14611 has been marked as a duplicate of this bug. ***