fix(solid_queue): don't perform the job twice when the guard clause raises

sentry-solid_queue/lib/sentry/solid_queue/active_job_extensions.rb

@@ -14,28 +14,36 @@ def perform_now
 
         job_executed = false
 
-        # Worker threads (deserialized from queue): clone hub for thread isolation,
-        # then operate directly on the scope (like Sidekiq's server middleware).
-        # No with_scope — avoids double Scope#dup overhead.
-        #
-        # Inline perform_now (e.g., from a controller): use with_scope to
-        # preserve the existing request scope/transaction.
-        if @_solid_queue_worker_thread
-          Sentry.clone_hub_to_current_thread
-          scope = Sentry.get_current_scope
-          perform_with_sentry(scope) { job_executed = true; super }
-        else
-          Sentry.with_scope do |scope|
+        # NOTE: the rescue below MUST stay scoped to this begin block. As a
+        # method-level `rescue` it also covered the guard clause above, where
+        # `job_executed` is still nil — so an exception raised by the guard's
+        # own `super` was swallowed and the job was performed a second time on
+        # the same instance (executions == 2, duplicated side effects, and the
+        # *second* run's error propagating in place of the real one).
+        begin
+          # Worker threads (deserialized from queue): clone hub for thread isolation,
+          # then operate directly on the scope (like Sidekiq's server middleware).
+          # No with_scope — avoids double Scope#dup overhead.
+          #
+          # Inline perform_now (e.g., from a controller): use with_scope to
+          # preserve the existing request scope/transaction.
+          if @_solid_queue_worker_thread
+            Sentry.clone_hub_to_current_thread
+            scope = Sentry.get_current_scope
             perform_with_sentry(scope) { job_executed = true; super }
+          else
+            Sentry.with_scope do |scope|
+              perform_with_sentry(scope) { job_executed = true; super }
+            end
           end
+        rescue => e
+          # If the job already executed, the exception is a re-raised job error —
+          # let it propagate. If not, Sentry setup failed before the job ran,
+          # so fall back to running the job without instrumentation.
+          raise if job_executed
+          Sentry.sdk_logger.error("sentry-solid_queue failed to instrument job: #{e.message}") rescue nil
+          super
         end
-      rescue => e
-        # If the job already executed, the exception is a re-raised job error —
-        # let it propagate. If not, Sentry setup failed before the job ran,
-        # so fall back to running the job without instrumentation.
-        raise if job_executed
-        Sentry.sdk_logger.error("sentry-solid_queue failed to instrument job: #{e.message}") rescue nil
-        super
       end
 
       # --- Client-side: inject trace headers on enqueue ---

sentry-solid_queue/spec/sentry/solid_queue/active_job_extensions_spec.rb

@@ -11,6 +11,16 @@
         result = HappyJob.perform_now
         expect(result).to eq("happy")
       end
+
+      it "performs a raising job exactly once and propagates its own error" do
+        CountingSadJob.runs = 0
+        job = CountingSadJob.new
+
+        expect { job.perform_now }.to raise_error(RuntimeError, "counted failure")
+
+        expect(CountingSadJob.runs).to eq(1)
+        expect(job.executions).to eq(1)
+      end
     end
 
     context "when Sentry is initialized without tracing" do
@@ -24,6 +34,20 @@
         expect(event.exception.values.first.type).to eq("RuntimeError")
       end
 
+      # The fully instrumented path: guard passes, so the job runs inside
+      # perform_with_sentry. A raising job must still be performed once, and be
+      # reported once — a duplicate run would also double the Sentry event.
+      it "performs a raising instrumented job exactly once and reports it once" do
+        CountingSadJob.runs = 0
+        job = CountingSadJob.new
+
+        expect { job.perform_now }.to raise_error(RuntimeError, "counted failure")
+
+        expect(CountingSadJob.runs).to eq(1)
+        expect(job.executions).to eq(1)
+        expect(transport.events.count).to eq(1)
+      end
+
       it "does not capture events for successful jobs" do
         HappyJob.perform_now
 
@@ -44,6 +68,17 @@
         expect(transport.events.count).to eq(0)
       end
 
+      it "performs a raising non-SolidQueue job exactly once and propagates its own error" do
+        CountingNonSolidQueueSadJob.runs = 0
+        job = CountingNonSolidQueueSadJob.new
+
+        expect { job.perform_now }.to raise_error(RuntimeError, "counted failure")
+
+        expect(CountingNonSolidQueueSadJob.runs).to eq(1)
+        expect(job.executions).to eq(1)
+        expect(transport.events.count).to eq(0)
+      end
+
       it "sets the mechanism to solid_queue" do
         expect { SadJob.perform_now }.to raise_error(RuntimeError)
 
@@ -200,28 +235,42 @@
 
       it "falls back to uninstrumented execution if Sentry errors before the job runs" do
         allow(Sentry).to receive(:clone_hub_to_current_thread).and_raise(StandardError, "sentry boom")
+        CountingHappyJob.runs = 0
 
-        job = HappyJob.new
+        job = CountingHappyJob.new
         job_data = job.serialize
         job_data["_sentry"] = {}
         job.deserialize(job_data)
 
         # Should not raise — falls back to super
         result = job.perform_now
+
         expect(result).to eq("happy")
+        # The fallback runs the job; it must not *re-run* one that already ran.
+        expect(CountingHappyJob.runs).to eq(1)
+        expect(job.executions).to eq(1)
       end
 
       it "re-raises job errors even when safety rescue is active" do
-        # The outer rescue must not swallow actual job exceptions
-        expect { simulate_worker_perform(SadJob) }.to raise_error(RuntimeError, "I'm sad!")
+        # The outer rescue must not swallow actual job exceptions, and must not
+        # mistake a job error for an instrumentation failure and retry the job.
+        CountingSadJob.runs = 0
+
+        expect { simulate_worker_perform(CountingSadJob) }
+          .to raise_error(RuntimeError, "counted failure")
+
+        expect(CountingSadJob.runs).to eq(1)
       end
 
       it "falls back to uninstrumented execution on inline path if Sentry errors before the job runs" do
         allow(Sentry).to receive(:with_scope).and_raise(StandardError, "scope boom")
+        CountingHappyJob.runs = 0
 
         # Inline path (no deserialization) — should fall back to super
-        result = HappyJob.perform_now
+        result = CountingHappyJob.perform_now
+
         expect(result).to eq("happy")
+        expect(CountingHappyJob.runs).to eq(1)
       end
     end
 

sentry-solid_queue/spec/spec_helper.rb

@@ -130,6 +130,60 @@ def perform
   end
 end
 
+# Jobs that count their own executions.
+#
+# `perform_now` is a wrapper, so the invariant that matters is *how many times*
+# it calls the wrapped job — not what the job returns. Asserting only on the
+# return value or the error class is blind to a duplicate run: an idempotent
+# job produces an identical outcome either way, which is how a method-level
+# rescue managed to perform every job twice without failing a single example.
+# Use these fixtures, and assert on `runs`, wherever a path could double up.
+#
+# CountingSadJob and CountingNonSolidQueueSadJob also cover the guard's two
+# early-return paths — Sentry uninitialized, and a non-solid_queue adapter.
+class CountingSadJob < ActiveJob::Base
+  self.queue_adapter = :solid_queue
+
+  class << self
+    attr_accessor :runs
+  end
+  self.runs = 0
+
+  def perform
+    self.class.runs += 1
+    raise "counted failure"
+  end
+end
+
+class CountingNonSolidQueueSadJob < ActiveJob::Base
+  self.queue_adapter = :async
+
+  class << self
+    attr_accessor :runs
+  end
+  self.runs = 0
+
+  def perform
+    self.class.runs += 1
+    raise "counted failure"
+  end
+end
+
+# Succeeds, so only the counter can tell one run from two.
+class CountingHappyJob < ActiveJob::Base
+  self.queue_adapter = :solid_queue
+
+  class << self
+    attr_accessor :runs
+  end
+  self.runs = 0
+
+  def perform
+    self.class.runs += 1
+    "happy"
+  end
+end
+
 class RetryableJob < ActiveJob::Base
   self.queue_adapter = :solid_queue
   retry_on RuntimeError, wait: 0, attempts: 3