Skip to content

Commit

Permalink
fix: gracefully handle execution of webhooks that are deleted between…
Browse files Browse the repository at this point in the history
… execution attempts (#613)

PACT-1058
  • Loading branch information
bethesque authored Jun 7, 2023
1 parent c61eccc commit 1127b41
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 9 deletions.
23 changes: 16 additions & 7 deletions lib/pact_broker/webhooks/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,15 @@ def perform_with_connection(data)
def perform_with_triggered_webhook
@error_count = data[:error_count] || 0
begin
webhook_execution_result = PactBroker::Webhooks::TriggerService.execute_triggered_webhook_now(triggered_webhook, webhook_options(data))
if webhook_execution_result.success?
handle_success
if triggered_webhook.webhook
webhook_execution_result = PactBroker::Webhooks::TriggerService.execute_triggered_webhook_now(triggered_webhook, webhook_options(data))
if webhook_execution_result.success?
handle_success
else
handle_failure
end
else
handle_failure
handle_webhook_deleted
end
rescue StandardError => e
handle_error e
Expand Down Expand Up @@ -73,19 +77,24 @@ def handle_error e
end

def handle_success
update_triggered_webhook_status TriggeredWebhook::STATUS_SUCCESS
update_triggered_webhook_status(TriggeredWebhook::STATUS_SUCCESS)
end

def handle_failure
if reschedule_job?
reschedule_job
update_triggered_webhook_status TriggeredWebhook::STATUS_RETRYING
update_triggered_webhook_status(TriggeredWebhook::STATUS_RETRYING)
else
logger.info "Failed to execute webhook #{triggered_webhook.webhook_uuid} after #{retry_schedule.size + 1} attempts."
update_triggered_webhook_status TriggeredWebhook::STATUS_FAILURE
update_triggered_webhook_status(TriggeredWebhook::STATUS_FAILURE)
end
end

def handle_webhook_deleted
logger.info("Webhook with uuid #{triggered_webhook.webhook_uuid} cannot be executed it has been deleted. Marking triggered webhook as failed.")
update_triggered_webhook_status(TriggeredWebhook::STATUS_FAILURE)
end

def reschedule_job?
error_count < retry_schedule.size
end
Expand Down
25 changes: 23 additions & 2 deletions spec/lib/pact_broker/webhooks/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ module Webhooks
end

let(:base_url) { "http://broker" }
let(:triggered_webhook) { instance_double("PactBroker::Webhooks::TriggeredWebhook", webhook_uuid: "1234", id: 1) }
let(:triggered_webhook) { instance_double("PactBroker::Webhooks::TriggeredWebhook", webhook_uuid: "1234", id: 1, webhook: webhook) }
let(:webhook) { double("webhook") }
let(:result) { instance_double("PactBroker::Domain::WebhookExecutionResult", success?: success) }
let(:webhook_execution_configuration) do
instance_double(PactBroker::Webhooks::ExecutionConfiguration, retry_schedule: retry_schedule, to_hash: webhook_execution_configuration_hash)
Expand Down Expand Up @@ -172,7 +173,7 @@ module Webhooks
end
end

context "when the webhook gets deleted between executions" do
context "when the triggered webhook gets deleted between executions" do
before do
allow(PactBroker::Webhooks::TriggeredWebhook).to receive(:find).and_return(nil)
end
Expand All @@ -183,6 +184,26 @@ module Webhooks
subject
end
end

context "when the webhook gets deleted between executions" do
let(:webhook) { nil }

it "updates the triggered_webhook status to 'failure'" do
expect(PactBroker::Webhooks::Service).to receive(:update_triggered_webhook_status)
.with(triggered_webhook, TriggeredWebhook::STATUS_FAILURE)
subject
end

it "logs a message" do
expect(logger).to receive(:info).with("Webhook with uuid 1234 cannot be executed it has been deleted. Marking triggered webhook as failed.")
subject
end

it "does not reschedule the job" do
expect(Job).to_not receive(:perform_in)
subject
end
end
end
end
end

0 comments on commit 1127b41

Please sign in to comment.