diff --git a/enterprise/app/jobs/onboarding/help_center_article_writer_job.rb b/enterprise/app/jobs/onboarding/help_center_article_writer_job.rb index 68c218a19..c684bd632 100644 --- a/enterprise/app/jobs/onboarding/help_center_article_writer_job.rb +++ b/enterprise/app/jobs/onboarding/help_center_article_writer_job.rb @@ -1,6 +1,21 @@ class Onboarding::HelpCenterArticleWriterJob < ApplicationJob queue_as :low + # Catch-all so no exception type can wedge the generation in "generating". + # Declared FIRST because ActiveJob searches rescue handlers bottom-to-top: + # this puts StandardError at the bottom of the search order, so the specific + # retry_on/discard_on handlers declared below match first for their types. + # + # Without this, any error that isn't FirecrawlError or ArticleBuildFailed + # (e.g. ActiveRecord::RecordInvalid, SSL errors) falls through to ActiveJob's + # default retries, exhausts them, and lands in the dead set without ever + # calling finalize -> state stays "generating" at total - 1 until the 7-day + # Redis TTL expires. on_writer_failure logs the error, so code bugs are still + # visible; it just also progresses the state. + discard_on StandardError do |job, error| + job.send(:on_writer_failure, error) + end + retry_on Firecrawl::FirecrawlError, wait: :polynomially_longer, attempts: 3 do |job, error| job.send(:on_writer_failure, error) end diff --git a/spec/enterprise/jobs/onboarding/help_center_article_writer_job_spec.rb b/spec/enterprise/jobs/onboarding/help_center_article_writer_job_spec.rb index b01ec35e3..ff7b434da 100644 --- a/spec/enterprise/jobs/onboarding/help_center_article_writer_job_spec.rb +++ b/spec/enterprise/jobs/onboarding/help_center_article_writer_job_spec.rb @@ -102,6 +102,47 @@ RSpec.describe Onboarding::HelpCenterArticleWriterJob do end end + describe 'catch-all failure handling' do + # Any exception the job does not specifically handle (e.g. + # ActiveRecord::RecordInvalid from articles.create!, SSL errors, OOM) + # must still finalize the generation so state cannot wedge in + # "generating" at total - 1 until the Redis TTL expires. + + it 'increments the counter on an unhandled StandardError without re-raising' do + allow(Onboarding::HelpCenterArticleBuilder).to receive(:new).and_raise( + StandardError, 'unexpected boom' + ) + + expect { described_class.perform_now(*job_args) }.not_to raise_error + + expect(Onboarding::HelpCenterGenerationState.current(generation_id)).to include('finished' => '1') + end + + it 'marks generation completed when the final writer fails with an unhandled error' do + allow(Onboarding::HelpCenterArticleBuilder).to receive(:new).and_raise( + StandardError, 'unexpected boom' + ) + Onboarding::HelpCenterGenerationState.record_article_finished(generation_id) + + described_class.perform_now(*job_args) + + expect(Onboarding::HelpCenterGenerationState.current(generation_id)).to include( + 'status' => 'completed', 'finished' => '2' + ) + end + + it 'logs the failure so the error is not silent' do + allow(Onboarding::HelpCenterArticleBuilder).to receive(:new).and_raise( + StandardError, 'unexpected boom' + ) + allow(Rails.logger).to receive(:warn) + + described_class.perform_now(*job_args) + + expect(Rails.logger).to have_received(:warn).with(/gen=#{generation_id} failed: StandardError unexpected boom/) + end + end + describe 'missing state' do let(:built_article) { instance_double(Article, id: 9876) }