mirror of
https://github.com/discourse/discourse.git
synced 2026-09-05 04:40:41 -05:00
FIX: Destroy pending PM after an unexpected send error (#40461)
Previously, when `Scriptable::Utils.send_pm` raised an unexpected error, `Jobs::DiscourseAutomation::Tracker#send_pending_pm` only logged it and left the `PendingPm` row intact, so the scheduled job re-processed the same failing PM every minute indefinitely. This change destroys the pending PM in the generic `rescue` block — mirroring the existing `ActiveRecord::RecordNotSaved` handling — so a permanently-failing PM is removed after one attempt instead of looping forever. Resolves https://patch.discourse.org/patch-triage/14 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
0bad05d036
commit
6e69e6029d
@@ -56,6 +56,7 @@ module Jobs
|
||||
::DiscourseAutomation::Logger.error(
|
||||
"Error sending pending PM '#{pending_pm.title}': #{e.message}",
|
||||
)
|
||||
pending_pm.destroy!
|
||||
end
|
||||
|
||||
def run_pending_automation(pending_automation)
|
||||
|
||||
@@ -141,6 +141,34 @@ describe Jobs::DiscourseAutomation::Tracker do
|
||||
end
|
||||
end
|
||||
|
||||
context "when send_pm raises an unexpected error" do
|
||||
before { pending_pm.update!(execute_at: 2.hours.ago) }
|
||||
|
||||
it "destroys the pending pm so it isn't retried forever" do
|
||||
DiscourseAutomation::Scriptable::Utils.stubs(:send_pm).raises(
|
||||
StandardError.new("unexpected error"),
|
||||
)
|
||||
|
||||
expect { Jobs::DiscourseAutomation::Tracker.new.execute }.to change {
|
||||
automation.pending_pms.count
|
||||
}.by(-1)
|
||||
end
|
||||
end
|
||||
|
||||
context "when send_pm raises RecordNotSaved" do
|
||||
before { pending_pm.update!(execute_at: 2.hours.ago) }
|
||||
|
||||
it "destroys the pending pm" do
|
||||
DiscourseAutomation::Scriptable::Utils.stubs(:send_pm).raises(
|
||||
ActiveRecord::RecordNotSaved.new("not saved"),
|
||||
)
|
||||
|
||||
expect { Jobs::DiscourseAutomation::Tracker.new.execute }.to change {
|
||||
automation.pending_pms.count
|
||||
}.by(-1)
|
||||
end
|
||||
end
|
||||
|
||||
it "doesn't send multiple messages if the job is invoked multiple times concurrently" do
|
||||
pending_pm.update!(execute_at: 1.hour.from_now)
|
||||
expect do
|
||||
|
||||
Reference in New Issue
Block a user