feat: add catch-all failure handling to article writer (#14881)
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`.
<details><summary>
##### Script to remove dead jobs
</summary>
<p>
```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
````
</p>
</details>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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) }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user