Skip to content

Commit

Permalink
Improve error logging for message batch dispatching
Browse files Browse the repository at this point in the history
closes VICE-2941
flag=none

Test Plan:
 - tests pass
 - verify post merge in failed job error stack

Change-Id: I0b1913e78fb5791ce6bb76b56e0095683c4466a2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/293899
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Chawn Neal <[email protected]>
Product-Review: Chawn Neal <[email protected]>
QA-Review: Caleb Guanzon <[email protected]>
  • Loading branch information
drakeaharper committed Jun 23, 2022
1 parent ea027bd commit 8f3f9f1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/message_dispatcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
#

class MessageDispatcher < Delayed::PerformableMethod
class MessagesInBatchNotFound < StandardError
end

DeliverWorker = Struct.new(:message) do
def perform
message.for_queue.deliver
Expand Down Expand Up @@ -88,12 +91,13 @@ def self.deliver_batch(messages)
message_ids << m.id
previous_time = m.created_at
end
raise ActiveRecord::RecordNotFound unless messages.length == queued.length
raise MessagesInBatchNotFound, "IDs not found: #{queued.map(&:id) - messages.map(&:id)}" unless messages.length == queued.length
end
messages.each do |message|
message.deliver
rescue
# this delivery failed, we'll have to make an individual job to retry

dispatch(message)
end
end
Expand Down
11 changes: 11 additions & 0 deletions spec/lib/message_dispatcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@
@messages = (0...3).map { message_model(dispatch_at: Time.now, workflow_state: "staged", to: "somebody", updated_at: Time.now.utc - 11.minutes, user: user_factory, path_type: "email") }
end

it "shows message ids not found in batch process" do
messages = @messages
messages.push(Message.new(id: -1, created_at: Time.zone.now))
track_jobs { MessageDispatcher.batch_dispatch(messages) }
expect(created_jobs.size).to eq 1
job = created_jobs.first
run_jobs
job.reload
expect(job.last_error).to include("IDs not found: [-1]")
end

it "reschedules on Mailer delivery error, but not on canceled Message" do
track_jobs { MessageDispatcher.batch_dispatch(@messages) }
expect(created_jobs.size).to eq 1
Expand Down

0 comments on commit 8f3f9f1

Please sign in to comment.