Skip to content

Commit

Permalink
Set default retry behaviour (same as DelayedJob for now)
Browse files Browse the repository at this point in the history
  • Loading branch information
francois-ferrandis committed Feb 15, 2023
1 parent 6260356 commit e45c43a
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 12 deletions.
3 changes: 2 additions & 1 deletion app/jobs/application_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

class ApplicationJob < ActiveJob::Base
include DefaultJobBehaviour

queue_as :default
queue_with_priority 0
end
24 changes: 24 additions & 0 deletions app/jobs/concerns/default_job_behaviour.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

module DefaultJobBehaviour
extend ActiveSupport::Concern

included do
# This retry_on means:
# "retry 20 times with an exponential backoff, then mark job as discarded"
#
# Exponential backoff is n^4, so wait times between retries will be as follows:
# attempt: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
# backoff: 1s, 16s, 81s, 4m, 10m, 21m, 40m, 68m, 109m, 166m, 4h, 6h, 8h, 11h, 14h, 18h, 23h, 29h, 36h, 44h
retry_on(StandardError, wait: :exponentially_longer, attempts: 20)

# Makes sure every failed attempt is logged to Sentry
# (see: https://github.com/bensheldon/good_job#retries)
around_perform do |_job, block|
block.call
rescue StandardError => e
Sentry.capture_exception(e)
raise # will be caught by the retry mechanism
end
end
end
6 changes: 5 additions & 1 deletion app/jobs/webhook_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ class WebhookJob < ApplicationJob

queue_as :webhook

retry_on(OutgoingWebhookError, wait: :exponentially_longer, attempts: 10, queue: :webhook_retries) do |_job, error|
Sentry.capture_exception(error)
end

def perform(payload, webhook_endpoint_id)
webhook_endpoint = WebhookEndpoint.find(webhook_endpoint_id)

Expand Down Expand Up @@ -41,7 +45,7 @@ def perform(payload, webhook_endpoint_id)
# c'est en général lié à une mise à jour
# ou une suppression qui ne fonctionne pas
#
# Ce petit paliatif est là en attendant qu'ils
# Ce petit palliatif est là en attendant qu'ils
# fassent évoluer leur système.
def self.false_negative_from_drome?(body)
body = JSON.parse(body)
Expand Down
2 changes: 2 additions & 0 deletions app/mailers/custom_mailer_delivery_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

# See https://www.bigbinary.com/blog/rails-5-2-allows-mailers-to-use-custom-active-job-class
class CustomMailerDeliveryJob < ActionMailer::MailDeliveryJob
include DefaultJobBehaviour

# Only discard DeserializationError if it is caused by a ActiveRecord::RecordNotFound.
# We don't want to discard a job when deserialization failed because of a DB failure for example.
rescue_from ActiveJob::DeserializationError do |exception|
Expand Down
3 changes: 2 additions & 1 deletion config/initializers/good_job.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# frozen_string_literal: true

Rails.application.configure do
# Configure options individually...
config.active_job.default_priority = 0

config.good_job.preserve_job_records = true
config.good_job.retry_on_unhandled_error = false
config.good_job.on_thread_error = ->(exception) { Sentry.capture_exception(exception) }
Expand Down
36 changes: 30 additions & 6 deletions spec/jobs/webhook_job_spec.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,40 @@
# frozen_string_literal: false

describe WebhookJob, type: :job do
stub_sentry_events

describe "#perform" do
it "raises OutgoingWebhookError error with the correct error message" do
payload = "{}"
webhook_endpoint = create(:webhook_endpoint, secret: "bla", target_url: "https://example.com/rdv-s-endpoint")
let(:payload) { "{}" }
let(:webhook_endpoint) { create(:webhook_endpoint, secret: "bla", target_url: "https://example.com/rdv-s-endpoint") }

before do
stub_request(:post, "https://example.com/rdv-s-endpoint").and_return({ status: 500, body: "ERROR" })
end

context "when running the job for the first time" do
it "retries and sends info to Sentry" do
expect do
described_class.perform_now(payload, webhook_endpoint.id)
end.to have_enqueued_job(described_class).with(payload, webhook_endpoint.id).on_queue(:webhook_retries)
# expect(sentry_events).to be_empty
expect(sentry_events.last.exception.values.first.value).to match(/Webhook-Failure\s\(ERROR\):/)
expect(sentry_events.last.exception.values.first.type).to eq("OutgoingWebhookError")
end
end

expect do
described_class.perform_now(payload, webhook_endpoint.id)
end.to raise_error(OutgoingWebhookError, /Webhook-Failure\s\(ERROR\):/)
context "when running the job for the 10th time" do
before do
allow_any_instance_of(described_class).to receive(:executions_for).with([OutgoingWebhookError]).and_return(10) # rubocop:disable RSpec/AnyInstance
end

it "does not retry but sends notification to Sentry" do
expect do
described_class.perform_now(payload, webhook_endpoint.id)
end.not_to have_enqueued_job

expect(sentry_events.last.exception.values.first.value).to match(/Webhook-Failure\s\(ERROR\):/)
expect(sentry_events.last.exception.values.first.type).to eq("OutgoingWebhookError")
end
end
end

Expand Down
6 changes: 3 additions & 3 deletions spec/sms/sms_netsize_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
let(:user) { create(:user, phone_number: "+33601020304") }
let(:rdv) { create(:rdv, organisation: organisation, users: [user]) }

around { |example| perform_enqueued_jobs { example.run } }

stub_sentry_events

it "calls netsize API" do
stub_netsize_ok

Users::RdvSms.rdv_created(rdv, rdv.users.first, "t0k3n").deliver_later
perform_enqueued_jobs

valid_request = lambda do |req|
body = URI.decode_www_form(req.body).to_h
Expand All @@ -32,7 +31,8 @@ def expect_error_to_be_logged
expect do
expect do
Users::RdvSms.rdv_created(rdv, rdv.users.first, "t0k3n").deliver_later
end.to raise_error(SmsSender::SmsSenderFailure)
perform_enqueued_jobs
end.to change { sentry_events.last&.exception&.values&.first&.type }.from(nil).to("SmsSender::SmsSenderFailure")
end.to(change(Receipt, :count).by(1).and(change(sentry_events, :size).by(1)))
end

Expand Down

0 comments on commit e45c43a

Please sign in to comment.