Bug 14791 - Automatically attempt to resend failed notices
Summary: Automatically attempt to resend failed notices
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 14723
Blocks: 14767
  Show dependency treegraph
 
Reported: 2015-09-07 15:33 UTC by Lari Taskula
Modified: 2021-09-24 12:26 UTC (History)
3 users (show)

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


Attachments
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (11.20 KB, patch)
2015-09-09 13:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (11.14 KB, patch)
2015-09-09 13:21 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (11.54 KB, patch)
2015-09-09 14:25 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (11.70 KB, patch)
2015-09-09 14:36 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (12.16 KB, patch)
2015-09-18 10:40 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (12.14 KB, patch)
2015-09-18 10:41 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (12.00 KB, patch)
2017-04-24 16:32 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers (10.31 KB, patch)
2017-04-24 17:30 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 14791: Automatically resend SMS notices that failed due to network problems (8.62 KB, patch)
2019-04-08 15:16 UTC, Lari Taskula
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2015-09-07 15:33:07 UTC
Sometimes notices keep failing due to problems in network and they should be resent. Last week in my local library we experienced some network trouble and plenty of messages went into failed status. This can also be somewhat risky by manual SQL queries in production environment. 

Bug 12426 introduces a subroutine C4::Letters::ResendMessage($message_id) for resending failed messages.

Problem is that if we run this kind of feature as cronjob, we can end up in an infinite loop of sending the same messages over and over again and they will never be sent because of missing/invalid to_address or smsalertnumber. However, Bug 14590 focuses on setting valid messaging preferences for all users. With the feature presented in Bug 14590 we can assume that all notifications will have proper messaging preferences. 

If we do not want to rely on Bug 14590:

Another option would be to track messages that we already attempted to resend. We could try to resend each failed message for lets say 5 times each time increasing the time between last attempt. Once the message has been attempted to send 5 times, we will stop our attempts to resend it.
Comment 1 Lari Taskula 2015-09-08 10:26:42 UTC
(In reply to Lari Taskula from comment #0)
> Last week in my local library we experienced some network trouble
> and plenty of messages went into failed status. This can also be somewhat
> risky by manual SQL queries in production environment. 

Long day. What I meant to say was that currently the only way to resend failed messages without Bug 12426 is via manual database UPDATE queries to set them back into "pending" status, and this leaves some chance for it to go horribly wrong.

Anyway, I also though of a third option. We could use Koha::Exceptions introduced in Bug 13995 to throw an exception to C4::Letters in case of a connection failure. C4::Letters would catch this exception and leave the message in the queue untouched. In other words, the message would stay in pending status and would be attempted to send again by process_message_queue.pl.
Comment 2 Lari Taskula 2015-09-09 13:09:28 UTC Comment hidden (obsolete)
Comment 3 Lari Taskula 2015-09-09 13:21:38 UTC Comment hidden (obsolete)
Comment 4 Lari Taskula 2015-09-09 14:25:05 UTC Comment hidden (obsolete)
Comment 5 Lari Taskula 2015-09-09 14:36:24 UTC Comment hidden (obsolete)
Comment 6 Lari Taskula 2015-09-18 10:40:39 UTC Comment hidden (obsolete)
Comment 7 Lari Taskula 2015-09-18 10:41:58 UTC
Created attachment 42698 [details] [review]
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers
Comment 8 Chris Cormack 2016-06-03 13:06:43 UTC
This is dependent on 13995 which is currently failed QA
Comment 9 Lari Taskula 2017-04-24 16:32:19 UTC
Created attachment 62615 [details] [review]
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers

Sometimes notices keep failing due to various reasons. One common problem
is network connection failures. Because of this, the notices go into
'failed' status without another attempt for sending. This is very problematic,
because we have to not only monitor the failed messages but also resend
them manually. The purpose of this patch is to move us into more automated
way of handling delivery failures.

This patch enables us to handle exceptions in SMS messaging. The main idea
is to throw an exception from SMS::Send driver in case of a failure.
The exception will be caught by C4::SMS and from here it will be forwarded
to C4::Letters where instead of automatically setting the message into
'failed' status, we now can do as we wish with various exceptions.

As an example I have caught Koha::Exception::ConnectionFailed in
C4::Letters::_send_message_by_sms(). When we catch the said exception,
we simply leave the message in 'pending' status.  This way it will be resent
whenever process_message_queue.pl is executed again.

There are multiple other reasons of failure where Exceptions will come
in handy. For example the SMS Gateway provider may return some errors
at request, and with this patch we will be able to handle them better.

Below is a short example for making your SMS::Send driver throw an
exception in case of a connection failure in SMS/Send/MyDriver/Driver.pm.

_______________________________________________________________
use Koha::Exception::ConnectionFailed;

sub send_sms {
    #.....your implementation.....

    # Throw an exception in case of a connection error
    # $connError can be for example: ($curl->{'retcode'} == 6)
    # cURL code 6: CURLE_COULDNT_RESOLVE_HOST
    if ($connError){
        Koha::Exception::ConnectionFailed->throw(error => "Connection failed");
    }

    #.....your implementation.....
}
_______________________________________________________________

prerequisites:
-2. Set system preference SMSSendDriver to Example::ExceptionExample
-1. Enable system preference EnhancedMessagingPreferences

To test:
1. Have/create some pending sms messages into message_queue
2. Go to Patrons -> Notices
3. Observe that the your message is in pending status
4. Apply patch
5. Run misc/cronjob/process_message_queue.pl
6. Observe that your message is still in pending status

You can also test it with your own implementation of SMSSendDriver.
What you need to do is follow the example mentioned earlier to make
send_sms() subroutine throw Koha::Exception::ConnectionFailed
in case of a connection failure.
Comment 10 Lari Taskula 2017-04-24 17:30:24 UTC
Created attachment 62622 [details] [review]
Bug 14791: Resend failed notices - Add Koha::Exceptions to SMS::Send drivers

Sometimes notices keep failing due to various reasons. One common problem
is network connection failures. Because of this, the notices go into
'failed' status without another attempt for sending. This is very problematic,
because we have to not only monitor the failed messages but also resend
them manually. The purpose of this patch is to move us into more automated
way of handling delivery failures.

This patch enables us to handle exceptions in SMS messaging. The main idea
is to throw an exception from SMS::Send driver in case of a failure.
The exception will be caught by C4::SMS and from here it will be forwarded
to C4::Letters where instead of automatically setting the message into
'failed' status, we now can do as we wish with various exceptions.

As an example I have caught Koha::Exceptions::ConnectionFailed in
C4::Letters::_send_message_by_sms(). When we catch the said exception,
we simply leave the message in 'pending' status.  This way it will be resent
whenever process_message_queue.pl is executed again.

sub send_sms {
    #.....your implementation.....

    # Throw an exception in case of a connection error
    # $connError can be for example: ($curl->{'retcode'} == 6)
    # cURL code 6: CURLE_COULDNT_RESOLVE_HOST
    if ($connError){
        Koha::Exceptions::ConnectionFailed->throw(error => "Connection failed");
    }
}

To test:
1. prove t/db_dependent/Letters.t

Also testable with your own implementation of SMSSendDriver.
1. Have/create some pending sms messages into message_queue
2. Go to Patrons -> Notices
3. Observe that the your message is in pending status
4. Run misc/cronjob/process_message_queue.pl
5. Observe that the message is no longer pending
6. Add Koha::Exceptions::ConnectionFailed->throw(error => "Connection failed");
   to your SMS::Send driver send_sms.
7. Set your message back to pending status
8. Run misc/cronjob/process_message_queue.pl
9. Observe that your message is back in pending status
10. Observe it also has the following delivery note:
    "Connection failed. Attempting to resend."
Comment 11 Lari Taskula 2017-04-24 17:31:43 UTC
Removed dependency to Bug 13995 and rebased patch to use Koha::Exceptions instead. Switched to Needs Signoff
Comment 12 Katrin Fischer 2017-10-15 12:06:36 UTC
Depends on bug 14723 which currently doesn't apply. Setting to BLOCKED. Please change back to Needs Signoff once dependency has been resolved.
Comment 13 Lari Taskula 2019-04-08 15:16:15 UTC
Created attachment 87527 [details] [review]
Bug 14791: Automatically resend SMS notices that failed due to network problems

Sometimes notices keep failing due to various reasons. One common problem
is network connection failures. Because of this, the notices go into 'failed'
state. This is very problematic, because we have to not only monitor the failed
messages but also resend them manually. The purpose of this patch is to enable
more automated way of handling delivery failures.

This patch enables us to handle exceptions in SMS messaging. The main idea
is to throw an exception from SMS::Send driver in case of a failure.
The exception will be caught by C4::SMS and from here it will be forwarded
to C4::Letters, where instead of automatically setting the message into
'failed' status, we now can decide what to do with the message.

As an example I have caught Koha::Exceptions::ConnectionFailed in
C4::Letters::_send_message_by_sms(). When we catch the said exception,
we simply leave the message in 'pending' status.  This way it will be resent
whenever process_message_queue.pl is executed again.

sub send_sms {
    #.....your implementation.....

    # Throw an exception in case of a connection error
    # $connError can be for example: ($curl->{'retcode'} == 6)
    # cURL code 6: CURLE_COULDNT_RESOLVE_HOST
    if ($connError){
        Koha::Exceptions::ConnectionFailed->throw(error => "Connection failed");
    }
}

To test:
1. prove t/db_dependent/Letters.t

Also testable with your own implementation of SMSSendDriver.
1. Have/create some pending sms messages into message_queue
2. Go to Patrons -> Notices
3. Observe that the your message is in pending status
4. Run misc/cronjob/process_message_queue.pl
5. Observe that the message is no longer pending
6. Add Koha::Exceptions::ConnectionFailed->throw(error => "Connection failed");
   to your SMS::Send driver send_sms.
7. Set your message back to pending status
8. Run misc/cronjob/process_message_queue.pl
9. Observe that your message is back in pending status
10. Observe it also has the following delivery note:
    "Connection failed. Attempting to resend."

Sponsored-by: Hypernova Oy
Comment 14 Lari Taskula 2020-03-17 21:20:50 UTC
Blocked by Bug 14723.

I'm no longer able to work on this, so I'm setting assignee to default. Feel free to continue this work.
Comment 15 Owen Leonard 2021-09-24 12:26:09 UTC
(In reply to Lari Taskula from comment #14)
> Blocked by Bug 14723.

Bug 14723 is pushed to master, but this no longer applies.