diff --git a/CHANGELOG.md b/CHANGELOG.md index 49f6d2f09..35514d558 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ # Unreleased +* (bugfix) Retry GitHub server errors in background jobs with polynomial backoff for ten attempts + (about 4.3 hours plus jitter), retaining error reporting when retries are exhausted. # 0.45.5 * (bugfix) Update existing check runs without first attempting a duplicate insert. Concurrent diff --git a/app/jobs/shipit/background_job.rb b/app/jobs/shipit/background_job.rb index cef3c3cff..3a14093a1 100644 --- a/app/jobs/shipit/background_job.rb +++ b/app/jobs/shipit/background_job.rb @@ -8,8 +8,8 @@ class << self DEFAULT_RETRY_TIME_IN_SECONDS = 30 - # Write actions can sometimes fail intermittently, particulary for large and/or busy repositories - retry_on(Octokit::ServerError) + # Allow hours for GitHub outages to recover while still raising after the last attempt. + retry_on(Octokit::ServerError, wait: :polynomially_longer, attempts: 10) rescue_from(Octokit::TooManyRequests, Octokit::AbuseDetected) do |exception| retry_job wait: exception.response_headers.fetch("Retry-After", DEFAULT_RETRY_TIME_IN_SECONDS).to_i.seconds diff --git a/test/jobs/shipit/background_job_test.rb b/test/jobs/shipit/background_job_test.rb index d1e5ddfaa..daa313755 100644 --- a/test/jobs/shipit/background_job_test.rb +++ b/test/jobs/shipit/background_job_test.rb @@ -24,8 +24,29 @@ class BackgroundJobTest < ActiveSupport::TestCase end end + test "#perform backs off GitHub server errors and re-raises after ten attempts" do + Shipit.github.api.expects(:user).with(@user.github_id).times(10).raises(Octokit::InternalServerError) + job = BackgroundStubJob.new(@user).serialize + + freeze_time do + (1..9).each do |execution| + clear_enqueued_jobs + assert_enqueued_with(job: BackgroundStubJob, at: Time.now + (execution**4 + 2).seconds) do + ActiveJob::Base.execute(job) + end + job = enqueued_jobs.last + end + + clear_enqueued_jobs + assert_no_enqueued_jobs do + assert_raises(Octokit::InternalServerError) { ActiveJob::Base.execute(job) } + end + end + end + class BackgroundStubJob < BackgroundJob queue_as :default + self.retry_jitter = 0.0 def perform(user) Shipit.github.api.user(user.github_id) diff --git a/test/jobs/shipit/create_on_github_job_test.rb b/test/jobs/shipit/create_on_github_job_test.rb index 907b409b8..8937f408e 100644 --- a/test/jobs/shipit/create_on_github_job_test.rb +++ b/test/jobs/shipit/create_on_github_job_test.rb @@ -25,5 +25,37 @@ class CreateOnGithubJobTest < ActiveSupport::TestCase assert_nothing_raised { job.perform_now } end + + test "#perform retries failed deployment statuses without recreating published records" do + status = shipit_commit_deployment_statuses(:shipit2_deploy_third_in_progress) + deployment = status.commit_deployment + shipit_commit_deployment_statuses(:shipit2_deploy_third_failure).update!(github_id: 43) + response = stub(id: 44, url: "#{deployment.api_url}/statuses/44") + client_class = deployment.stack.github_api.class + client_class.any_instance.expects(:create_deployment).never + client_class.any_instance.expects(:create_deployment_status) + .with(deployment.api_url, 'in_progress', anything).twice + .raises(Octokit::InternalServerError).then.returns(response) + + assert_enqueued_with(job: CreateOnGithubJob) { CreateOnGithubJob.perform_now(deployment) } + assert_nil status.reload.github_id + + retry_job = enqueued_jobs.last + clear_enqueued_jobs + assert_no_enqueued_jobs { ActiveJob::Base.execute(retry_job) } + + assert_equal response.id, status.reload.github_id + assert_equal response.url, status.api_url + end + + test "#perform re-raises after exhausting GitHub server error retries" do + CommitDeployment.any_instance.stubs(:create_on_github!).raises(Octokit::InternalServerError) + job = CreateOnGithubJob.new(@deployment) + job.exception_executions = { "[Octokit::ServerError]" => 9 } + + assert_no_enqueued_jobs do + assert_raises(Octokit::InternalServerError) { job.perform_now } + end + end end end diff --git a/test/jobs/shipit/refresh_check_runs_job_test.rb b/test/jobs/shipit/refresh_check_runs_job_test.rb new file mode 100644 index 000000000..d8116cf4e --- /dev/null +++ b/test/jobs/shipit/refresh_check_runs_job_test.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'test_helper' + +module Shipit + class RefreshCheckRunsJobTest < ActiveSupport::TestCase + test "#perform backs off GitHub server errors" do + Commit.any_instance.stubs(:refresh_check_runs!).raises(Octokit::InternalServerError) + job = RefreshCheckRunsJob.new(commit_id: shipit_commits(:first).id) + job.exception_executions = { "[Octokit::ServerError]" => 1 } + + freeze_time do + retry_time = ->(at) { (18.seconds.from_now..20.4.seconds.from_now).cover?(at) } + assert_enqueued_with(job: RefreshCheckRunsJob, at: retry_time) { job.perform_now } + end + end + end +end