SmsStorageException in SmsMessage::setResult() causes repeated dispatch of duplicate messages after failed queue

Created on 9 September 2017, over 7 years ago

Updated 31 May 2023, almost 2 years ago

Problem Statement:

A slight inconsistency has been exposed in SmsMessage::setResult() which results in a message being dispatched multiple times in some situations.
This is due to the fact that SmsMessage::setResult() throws an SmsStorageException('Saved SMS message results cannot be changed or updated.') exception if there is already a result.

So in a scenario where a queued message is dispatched, but for some reason, some other exception causes the QueueWorker\SmsProcessor to exit without cleaning up, the queued item would be saved and still left in the queue. On the next cron run, another attempt will be made to re-send this message. Sending will succeed (another copy of same message sent), but the $sms->setResult() line will throw the exception, the QueueWorker\SmsProcessor will exit again without cleanup and the $sms_message will still be left in the queue to be dispatched on the next cron run again....and on and on.

Resolution:

This needs to be discussed, but a possible check would be to confirm that the $sms_message hasn't previously been sent ($sms->getResult() === null) before attempting to send again.

🐛 Bug report

Status

Active

Version

1.0

Component

Core Framework

Created by

🇳🇬Nigeria almaudoh

Live updates comments and jobs are added and updated live.

Incomplete comments

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Comment almost 2 years ago →
🇯🇴Jordan Anas_maw
Facing the same problem, this is a critical issue.
Comment about 2 months ago →
🇯🇴Jordan Manar Olimat
Comment about 2 months ago →
🇯🇴Jordan Manar Olimat
Comment about 1 month ago →
🇦🇺Australia dpi Perth, Australia
Is this because the exception is caught in the code above SmsProcessor in the queue runner? Drush, etc?

Perhaps SmsProcessor should catch and swallow SmsStorageException, or event better, an even narrower exception extending that?

I'm not sure if we can change the behaviour at this point.

For now I'm closing this issue in the name of 🌱 [policy] Announcing closure of 3.x issues if they are not applicable for V4 Active , however if a patch is devised with the rework I'll consider it. But the changes will not apply to the new versions of SMS Framework, only these legacy version.s
Comment about 1 month ago →
🇦🇺Australia dpi Perth, Australia

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024