From 143508fc221c6a28864153a1ac046b4f4ffc3d8d Mon Sep 17 00:00:00 2001 From: Joseph Hale Date: Mon, 2 Mar 2026 14:13:00 -0700 Subject: [PATCH 1/4] Skip AnalyzeJob when blob has no attachments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upload-then-delete races cause AnalyzeJob to hit S3 after PurgeJob has already removed the object, producing Aws::S3::Errors::NoSuchKey noise in solid_queue_failed_executions. Since PurgeOnLastAttachment destroys the attachment row before enqueueing PurgeJob, checking blob.attachments.exists? catches this — a fast DB query that avoids the S3 round-trip entirely. Co-Authored-By: Claude Opus 4.6 --- ...storage_analyze_job_suppress_broadcasts.rb | 2 ++ ...ge_analyze_job_suppress_broadcasts_test.rb | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb diff --git a/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb b/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb index ba22603b6..030232ee4 100644 --- a/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb +++ b/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb @@ -4,6 +4,8 @@ # there is currently a bug https://github.com/rails/rails/issues/55144 module ActiveStorageAnalyzeJobSuppressBroadcasts def perform(blob) + return unless blob.attachments.exists? + Board.suppressing_turbo_broadcasts do Card.suppressing_turbo_broadcasts do super diff --git a/test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb b/test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb new file mode 100644 index 000000000..1015c957c --- /dev/null +++ b/test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb @@ -0,0 +1,23 @@ +require "test_helper" + +class ActiveStorageAnalyzeJobSuppressBroadcastsTest < ActiveSupport::TestCase + test "skips analysis when blob has no attachments" do + blob = ActiveStorage::Blob.create_and_upload!( + io: StringIO.new("x" * 1024), filename: "orphan.txt", content_type: "text/plain" + ) + + blob.expects(:analyze).never + + ActiveStorage::AnalyzeJob.perform_now(blob) + end + + test "performs analysis when blob has attachments" do + card = cards(:logo) + card.image.attach io: StringIO.new("x" * 1024), filename: "test.png", content_type: "image/png" + blob = card.image.blob + + blob.expects(:analyze).once + + ActiveStorage::AnalyzeJob.perform_now(blob) + end +end From c61f49cce97560dde49d00c66dca4df618e69374 Mon Sep 17 00:00:00 2001 From: Joseph Hale Date: Mon, 2 Mar 2026 14:38:19 -0700 Subject: [PATCH 2/4] Extract detached-blob guard into its own file Separate the attachment existence check from the broadcast suppression override so each file has a single responsibility. The guard now lives in ActiveStorageAnalyzeJobSkipDetached with its own documentation explaining the upload-then-delete race condition. Co-Authored-By: Claude Opus 4.6 --- .../active_storage_analyze_job_skip_detached.rb | 17 +++++++++++++++++ ...e_storage_analyze_job_suppress_broadcasts.rb | 2 -- ...e_storage_analyze_job_skip_detached_test.rb} | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 lib/rails_ext/active_storage_analyze_job_skip_detached.rb rename test/lib/rails_ext/{active_storage_analyze_job_suppress_broadcasts_test.rb => active_storage_analyze_job_skip_detached_test.rb} (88%) diff --git a/lib/rails_ext/active_storage_analyze_job_skip_detached.rb b/lib/rails_ext/active_storage_analyze_job_skip_detached.rb new file mode 100644 index 000000000..efa671335 --- /dev/null +++ b/lib/rails_ext/active_storage_analyze_job_skip_detached.rb @@ -0,0 +1,17 @@ +# Skip analysis for blobs whose attachments have already been destroyed. +# +# When an upload is deleted before AnalyzeJob runs, PurgeOnLastAttachment has +# already destroyed the attachment row and enqueued PurgeJob for the blob's S3 +# object. Analyzing at this point hits a missing key. Same check as +# PurgeOnLastAttachment#purge_blob_if_last. +module ActiveStorageAnalyzeJobSkipDetached + def perform(blob) + return unless blob.attachments.exists? + + super + end +end + +ActiveSupport.on_load :active_storage_blob do + ActiveStorage::AnalyzeJob.prepend ActiveStorageAnalyzeJobSkipDetached +end diff --git a/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb b/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb index 030232ee4..ba22603b6 100644 --- a/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb +++ b/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts.rb @@ -4,8 +4,6 @@ # there is currently a bug https://github.com/rails/rails/issues/55144 module ActiveStorageAnalyzeJobSuppressBroadcasts def perform(blob) - return unless blob.attachments.exists? - Board.suppressing_turbo_broadcasts do Card.suppressing_turbo_broadcasts do super diff --git a/test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb b/test/lib/rails_ext/active_storage_analyze_job_skip_detached_test.rb similarity index 88% rename from test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb rename to test/lib/rails_ext/active_storage_analyze_job_skip_detached_test.rb index 1015c957c..5ab5edaff 100644 --- a/test/lib/rails_ext/active_storage_analyze_job_suppress_broadcasts_test.rb +++ b/test/lib/rails_ext/active_storage_analyze_job_skip_detached_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class ActiveStorageAnalyzeJobSuppressBroadcastsTest < ActiveSupport::TestCase +class ActiveStorageAnalyzeJobSkipDetachedTest < ActiveSupport::TestCase test "skips analysis when blob has no attachments" do blob = ActiveStorage::Blob.create_and_upload!( io: StringIO.new("x" * 1024), filename: "orphan.txt", content_type: "text/plain" From 7611236a67a61afae4688924a7577843efa78022 Mon Sep 17 00:00:00 2001 From: Joseph Hale Date: Mon, 2 Mar 2026 14:40:21 -0700 Subject: [PATCH 3/4] Simplify comment in skip-detached override Co-Authored-By: Claude Opus 4.6 --- lib/rails_ext/active_storage_analyze_job_skip_detached.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/rails_ext/active_storage_analyze_job_skip_detached.rb b/lib/rails_ext/active_storage_analyze_job_skip_detached.rb index efa671335..e48154652 100644 --- a/lib/rails_ext/active_storage_analyze_job_skip_detached.rb +++ b/lib/rails_ext/active_storage_analyze_job_skip_detached.rb @@ -1,9 +1,5 @@ -# Skip analysis for blobs whose attachments have already been destroyed. -# -# When an upload is deleted before AnalyzeJob runs, PurgeOnLastAttachment has -# already destroyed the attachment row and enqueued PurgeJob for the blob's S3 -# object. Analyzing at this point hits a missing key. Same check as -# PurgeOnLastAttachment#purge_blob_if_last. +# Skip analysis for blobs whose attachments have already been destroyed. Avoids +# Aws::S3::Errors::NoSuchKey when an upload is deleted before AnalyzeJob runs. module ActiveStorageAnalyzeJobSkipDetached def perform(blob) return unless blob.attachments.exists? From 451920adc7ae3508fc04b17f34f89bd818b23958 Mon Sep 17 00:00:00 2001 From: Joseph Hale Date: Mon, 2 Mar 2026 15:25:11 -0700 Subject: [PATCH 4/4] Include concrete example motivating this module file --- lib/rails_ext/active_storage_analyze_job_skip_detached.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/rails_ext/active_storage_analyze_job_skip_detached.rb b/lib/rails_ext/active_storage_analyze_job_skip_detached.rb index e48154652..0df49f0cf 100644 --- a/lib/rails_ext/active_storage_analyze_job_skip_detached.rb +++ b/lib/rails_ext/active_storage_analyze_job_skip_detached.rb @@ -1,5 +1,6 @@ -# Skip analysis for blobs whose attachments have already been destroyed. Avoids -# Aws::S3::Errors::NoSuchKey when an upload is deleted before AnalyzeJob runs. +# Skip analysis for blobs whose attachments have already been destroyed. +# e.g. when a user uploads a file but deletes it before the analysis runs. +# Avoids `Aws::S3::Errors::NoSuchKey` when an upload is deleted before AnalyzeJob runs. module ActiveStorageAnalyzeJobSkipDetached def perform(blob) return unless blob.attachments.exists?