From b8545019e14889353cd60e85b36e18382879339d Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 2 Jul 2026 16:17:34 +0530 Subject: [PATCH] feat: add catch-all failure handling to article writer (#14881) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Help-center onboarding jobs were getting stuck in the `generating` state indefinitely. `Onboarding::HelpCenterArticleWriterJob` only finalized the generation counter for two exception types (`Firecrawl::FirecrawlError`, `ArticleBuildFailed`). Any other error exhausted Sidekiq's default retries and landed in the dead set without ever bumping `finished`, leaving state at `total - 1` / `generating` until the 7-day Redis TTL expired. Observed in production: 4 generations wedged at exactly `finished = total - 1`, four days after the run, with no further progress — the onboarding UI showed "generating" the whole time. This PR adds a `discard_on StandardError` catch-all to `HelpCenterArticleWriterJob`, after the existing `retry_on Firecrawl::FirecrawlError` and `discard_on ArticleBuildFailed` handlers. ActiveJob matches in declaration order, first match wins, so existing behavior is unchanged — the catch-all only absorbs errors that previously fell through to unhandled retry-then-dead-set. It routes through the same `on_writer_failure` → `finalize` path, so state always progresses to `completed`.
##### Script to remove dead jobs

```rb # One-off: unstick Onboarding::HelpCenterGenerationState keys that are wedged in # "generating" because a writer job died on an unhandled exception (neither # FirecrawlError nor ArticleBuildFailed) and exhausted Sidekiq retries without # ever calling record_article_finished. # # These portals have real articles (finished is 1 short of total). Marking them # "completed" is the honest terminal state: generation is done, one article failed. # # Run on a prod box (dry-run first, then REMOVE_DRY_RUN=1): # RAILS_ENV=production bundle exec rails runner scripts/help_center_investigation/unstick_generating.rb # # To actually write, set REMOVE_DRY_RUN=1 in the environment. pattern = format(Redis::Alfred::HELP_CENTER_GENERATION, id: '*') dry_run = ENV['REMOVE_DRY_RUN'].blank? stuck = [] Redis::Alfred.with do |conn| conn.scan_each(match: pattern, count: 1000) do |key| h = conn.hgetall(key) next unless h['status'] == 'generating' gen_id = key.sub('HELP_CENTER_GENERATION::', '') stuck << { gen_id: gen_id, total: h['total'], finished: h['finished'], key: key } unless dry_run conn.hset(key, 'status', 'completed') conn.expire(key, Onboarding::HelpCenterGenerationState::TTL) end end end puts "#{dry_run ? '[DRY RUN] ' : ''}Found #{stuck.size} stuck 'generating' states:" stuck.each do |s| puts " gen=#{s[:gen_id]} total=#{s[:total]} finished=#{s[:finished]} -> #{dry_run ? 'would mark completed' : 'marked completed'}" end puts puts 'Re-run with REMOVE_DRY_RUN=1 to apply.' if dry_run ````

--- .../help_center_article_writer_job.rb | 15 +++++++ .../help_center_article_writer_job_spec.rb | 41 +++++++++++++++++++ 2 files changed, 56 insertions(+) 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) }