Browse Source

Fixes #3416: Scheduler dies if Trigger, Ticket or Article of a Webhook job got deleted before job got executed.

Thorsten Eckel 4 years ago
parent
commit
1ac83882f7

+ 5 - 0
app/jobs/trigger_webhook_job.rb

@@ -17,6 +17,11 @@ class TriggerWebhookJob < ApplicationJob
     executions * 10.seconds
   }
 
+  discard_on(ActiveJob::DeserializationError) do |_job, e|
+    Rails.logger.info 'Trigger, Ticket or Article may got removed before TriggerWebhookJob could be executed. Discarding job. See exception for further details.'
+    Rails.logger.info e
+  end
+
   def perform(trigger, ticket, article)
     @trigger = trigger
     @ticket  = ticket

+ 12 - 3
config/initializers/delayed_jobs_ensure_active_job_lock_removal.rb

@@ -10,11 +10,20 @@ module Delayed
       return if !payload_object.is_a?(::ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper)
 
       # deserialize ActiveJob and load it's arguments to generate the lock_key
-      active_job           = ::ActiveJob::Base.deserialize(payload_object.job_data)
-      active_job.arguments = ::ActiveJob::Arguments.deserialize(active_job.instance_variable_get(:@serialized_arguments))
+      active_job = ::ActiveJob::Base.deserialize(payload_object.job_data)
+
+      # ActiveJob that is not an HasActiveJobLock has no lock
+      return if !active_job.is_a?(HasActiveJobLock)
+
+      begin
+        active_job.arguments = ::ActiveJob::Arguments.deserialize(active_job.instance_variable_get(:@serialized_arguments))
+      rescue => e
+        active_job.arguments = nil
+        Rails.logger.error e
+      end
 
       # remove possible lock
-      active_job.try(:release_active_job_lock!)
+      active_job.release_active_job_lock!
     end
   end
 end

+ 52 - 3
spec/jobs/trigger_webhook_job_spec.rb

@@ -1,6 +1,58 @@
 require 'rails_helper'
 
 RSpec.describe TriggerWebhookJob, type: :job do
+
+  let(:endpoint) { 'http://api.example.com/webhook' }
+  let(:token) { 's3cr3t-t0k3n' }
+
+  context 'when serialized model argument gets deleted' do
+
+    subject!(:job) { described_class.perform_later(trigger, ticket, article) }
+
+    let(:ticket) { create(:ticket) }
+    let(:article) { create(:'ticket/article') }
+
+    let(:trigger) do
+      create(:trigger,
+             perform: {
+               'notification.webhook' => {
+                 endpoint: endpoint,
+                 token:    token
+               }
+             })
+    end
+
+    shared_examples 'handle deleted argument models' do
+      it 'raises no error' do
+        expect { ActiveJob::Base.execute job.serialize }.not_to raise_error
+      end
+
+      it "doesn't perform request" do
+        allow(UserAgent).to receive(:post)
+        ActiveJob::Base.execute job.serialize
+        expect(UserAgent).not_to have_received(:post)
+      end
+    end
+
+    context 'when Trigger gets deleted' do
+      before { trigger.destroy! }
+
+      include_examples 'handle deleted argument models'
+    end
+
+    context 'when Ticket gets deleted' do
+      before { ticket.destroy! }
+
+      include_examples 'handle deleted argument models'
+    end
+
+    context 'when Article gets deleted' do
+      before { article.destroy! }
+
+      include_examples 'handle deleted argument models'
+    end
+  end
+
   describe '#perform' do
     subject(:perform) { described_class.perform_now(trigger, ticket, article) }
 
@@ -20,9 +72,6 @@ RSpec.describe TriggerWebhookJob, type: :job do
              })
     end
 
-    let(:endpoint) { 'http://api.example.com/webhook' }
-    let(:token) { 's3cr3t-t0k3n' }
-
     let(:response_status) { 200 }
     let(:payload) do
       {