fix(rails): capture handled Rails.error reports during web requests

.changeset/olive-cats-take.md

@@ -0,0 +1,10 @@
+---
+"posthog-rails": patch
+"posthog-ruby": patch
+---
+
+Capture handled errors reported through `Rails.error` during web requests by replacing Rails-specific suppression with shared exception deduplication.
+
+For mutable exceptions, `capture_exception` now suppresses subsequent captures of the same exception object and its captured causes, including manual calls and calls through another client. The first successful capture wins; subsequent calls return `false` without merging context. Supply the required identity and context on the first report. Failed captures remain retryable.
+
+Remove `PostHog::Rails::IN_WEB_REQUEST_KEY` and the request-tracking and exception-marker helpers documented as private. Remove any application references to these integration internals.

lib/posthog/client.rb

@@ -54,6 +54,9 @@ class Client
     RELEASE_ID_ENV_VAR = 'POSTHOG_RELEASE_ID'
     private_constant :RELEASE_ID_ENV_VAR
 
+    CAPTURED_EXCEPTION_IVAR = :@posthog_exception_captured
+    private_constant :CAPTURED_EXCEPTION_IVAR
+
     # Thread-safe tracking of client instances per API key for singleton warnings
     @instances_by_api_key = {}
     @instances_mutex = Mutex.new
@@ -410,7 +413,7 @@ def capture(attrs)
     #   Defaults to `{ 'type' => 'generic', 'handled' => true }` for manual captures.
     # @return [Boolean, nil] Whether the exception event was queued or sent, or nil if the input could not be parsed.
     def capture_exception(exception, distinct_id = nil, additional_properties = {}, flags: nil, mechanism: nil)
-      return false if @disabled
+      return false if @disabled || exception_captured?(exception)
 
       exception_list = ExceptionCapture.build_exception_list(exception, mechanism: mechanism)
 
@@ -427,7 +430,9 @@ def capture_exception(exception, distinct_id = nil, additional_properties = {},
       }
       event_data[:flags] = flags if flags
 
-      capture(event_data)
+      capture(event_data).tap do |captured|
+        mark_exception_captured(exception) if captured
+      end
     end
 
     # Identifies a user
@@ -978,6 +983,25 @@ def shutdown(timeout: nil)
 
     private
 
+    def exception_captured?(exception)
+      exception.is_a?(Exception) && exception.instance_variable_get(CAPTURED_EXCEPTION_IVAR).equal?(exception)
+    rescue StandardError
+      false
+    end
+
+    # Rails can unwrap template errors, so mark the causes included in the event too.
+    def mark_exception_captured(exception)
+      ExceptionCapture::MAX_CHAINED_EXCEPTIONS.times do
+        break unless exception.is_a?(Exception)
+        break if exception_captured?(exception)
+
+        exception.instance_variable_set(CAPTURED_EXCEPTION_IVAR, exception) unless exception.frozen?
+        exception = exception.cause
+      end
+    rescue StandardError
+      nil
+    end
+
     def enrich_capture_attrs_with_context(attrs)
       context = Internal::Context.current
       explicit_properties = attrs[:properties]
@@ -1152,10 +1176,7 @@ def enqueue(action)
       action = process_before_send(action)
       return false if action.nil? || action.empty?
 
-      if @sync_mode
-        send_sync(action)
-        return true
-      end
+      return send_sync(action) if @sync_mode
 
       queued = @queue_mutex.synchronize do
         if @queue.length < @max_queue_size
@@ -1223,13 +1244,15 @@ def send_sync(action)
         batch << action
       rescue MessageBatch::JSONGenerationError => e
         @on_error.call(-1, e.to_s)
-        return
+        return false
       end
-      return if batch.empty?
+      return false if batch.empty?
 
       @sync_lock.synchronize do
         res = @transport.send(@api_key, batch)
-        @on_error.call(res.status, res.error) unless res.status.between?(200, 299)
+        success = res.status.between?(200, 299)
+        @on_error.call(res.status, res.error) unless success
+        success
       end
     end
 

posthog-rails/lib/posthog/rails.rb

@@ -18,14 +18,6 @@
 
 module PostHog
   module Rails
-    # Thread-local key for tracking web request context
-    IN_WEB_REQUEST_KEY = :posthog_in_web_request
-    ACTIVE_JOB_CAPTURED_EXCEPTION_IVAR = :@posthog_active_job_exception_captured
-    WEB_CAPTURED_EXCEPTION_IVAR = :@posthog_web_exception_captured
-
-    private_constant :ACTIVE_JOB_CAPTURED_EXCEPTION_IVAR
-    private_constant :WEB_CAPTURED_EXCEPTION_IVAR
-
     class << self
       # @return [PostHog::Rails::Configuration] Rails integration configuration.
       def config
@@ -42,87 +34,6 @@ def config
       def configure
         yield config if block_given?
       end
-
-      # Mark that we're in a web request context
-      # CaptureExceptions middleware will handle exception capture
-      # @api private
-      # @return [void]
-      def enter_web_request
-        Thread.current[IN_WEB_REQUEST_KEY] = true
-      end
-
-      # Clear web request context (called at end of request)
-      # @api private
-      # @return [void]
-      def exit_web_request
-        Thread.current[IN_WEB_REQUEST_KEY] = false
-      end
-
-      # Check if we're currently in a web request context
-      # Used by ErrorSubscriber to avoid duplicate captures
-      # @api private
-      # @return [Boolean]
-      def in_web_request?
-        Thread.current[IN_WEB_REQUEST_KEY] == true
-      end
-
-      # Mark an exception as already captured by the ActiveJob integration.
-      # Used by ErrorSubscriber to avoid duplicate captures when Rails reports
-      # the same re-raised job exception via Rails.error.
-      # @api private
-      # @param exception [Exception]
-      # @return [void]
-      def mark_active_job_exception_captured(exception)
-        exception.instance_variable_set(ACTIVE_JOB_CAPTURED_EXCEPTION_IVAR, true)
-      rescue StandardError
-        nil
-      end
-
-      # Check whether an exception was already captured by ActiveJobExtensions.
-      # @api private
-      # @param exception [Exception]
-      # @return [Boolean]
-      def active_job_exception_captured?(exception)
-        exception.instance_variable_get(ACTIVE_JOB_CAPTURED_EXCEPTION_IVAR) == true
-      rescue StandardError
-        false
-      end
-
-      # Mark an exception as already captured by the CaptureExceptions middleware.
-      #
-      # ActionDispatch::Executor sits above our middleware and reports unhandled
-      # exceptions to Rails.error *after* the response has unwound back through
-      # CaptureExceptions, so `in_web_request?` is already false by then. The mark
-      # lets ErrorSubscriber recognize the report as a duplicate.
-      #
-      # The whole cause chain is marked because ActionDispatch::ExceptionWrapper
-      # unwraps ActionView::Template::Error to its cause, meaning Rails can report
-      # a different object than the one the middleware captured.
-      # @api private
-      # @param exception [Exception]
-      # @return [void]
-      def mark_web_exception_captured(exception)
-        current = exception
-        seen = {}.compare_by_identity
-
-        while current.is_a?(Exception) && !seen[current]
-          seen[current] = true
-          current.instance_variable_set(WEB_CAPTURED_EXCEPTION_IVAR, true)
-          current = current.cause
-        end
-      rescue StandardError
-        nil
-      end
-
-      # Check whether an exception was already captured by CaptureExceptions.
-      # @api private
-      # @param exception [Exception]
-      # @return [Boolean]
-      def web_exception_captured?(exception)
-        exception.instance_variable_get(WEB_CAPTURED_EXCEPTION_IVAR) == true
-      rescue StandardError
-        false
-      end
     end
   end
 end

posthog-rails/lib/posthog/rails/active_job.rb

@@ -70,7 +70,6 @@ def capture_job_exception(exception)
           properties,
           mechanism: { 'type' => 'active_job', 'handled' => false }
         )
-        PostHog::Rails.mark_active_job_exception_captured(exception)
       rescue StandardError => e
         # Don't let PostHog errors break job processing
         PostHog::Logging.logger.error("Failed to capture job exception: #{e.message}")

posthog-rails/lib/posthog/rails/capture_exceptions.rb

@@ -18,10 +18,6 @@ def initialize(app)
       # @param env [Hash] Rack environment.
       # @return [Array] Rack response.
       def call(env)
-        # Signal that we're in a web request context
-        # ErrorSubscriber will skip capture for web requests to avoid duplicates
-        PostHog::Rails.enter_web_request
-
         response = @app.call(env)
         env['posthog.response_status_code'] = response_status(response)
 
@@ -35,8 +31,6 @@ def call(env)
         # Capture unhandled exceptions
         capture_exception(e, env) if should_capture?(e)
         raise
-      ensure
-        PostHog::Rails.exit_web_request
       end
 
       private
@@ -60,13 +54,12 @@ def capture_exception(exception, env)
         distinct_id = extract_distinct_id(env)
         additional_properties = build_properties(request, env)
 
-        captured = PostHog.capture_exception(
+        PostHog.capture_exception(
           exception,
           distinct_id,
           additional_properties,
           mechanism: { 'type' => 'rails', 'handled' => false }
         )
-        PostHog::Rails.mark_web_exception_captured(exception) if captured
       rescue StandardError => e
         PostHog::Logging.logger.error("Failed to capture exception: #{e.message}")
         PostHog::Logging.logger.error("Backtrace: #{e.backtrace&.first(5)&.join("\n")}")

posthog-rails/lib/posthog/rails/error_subscriber.rb

@@ -20,15 +20,6 @@ class ErrorSubscriber
       def report(error, handled:, severity:, context:, source: nil)
         return unless PostHog::Rails.config&.auto_capture_exceptions
         return unless PostHog::Rails.config&.should_capture_exception?(error)
-        # Skip if in a web request - CaptureExceptions middleware will handle it
-        # with richer context (URL, params, controller, etc.)
-        return if PostHog::Rails.in_web_request?
-        # ActionDispatch::Executor reports the exception once the response has
-        # unwound past our middleware, so the flag above is already cleared.
-        # CaptureExceptions marks what it captured to catch that late report.
-        return if PostHog::Rails.web_exception_captured?(error)
-        return if PostHog::Rails.config&.auto_instrument_active_job &&
-                  PostHog::Rails.active_job_exception_captured?(error)
 
         distinct_id = context[:user_id] || context[:distinct_id]
 

public_api_snapshot.txt

@@ -327,7 +327,6 @@ instance_method PostHog::Rails::Configuration#use_tracing_headers=(arg0)
 instance_method PostHog::Rails::Configuration#user_id_method()
 instance_method PostHog::Rails::Configuration#user_id_method=(arg0)
 constant PostHog::Rails::Configuration::DEFAULT_LOGS_MAX_RECORDS_PER_MINUTE: Integer
-constant PostHog::Rails::IN_WEB_REQUEST_KEY: Symbol
 class PostHog::Rails::InitConfig
 instance_method PostHog::Rails::InitConfig#api_key=(value)
 instance_method PostHog::Rails::InitConfig#before_send=(value)

spec/posthog/client_spec.rb

@@ -274,11 +274,36 @@ def shutdown
         )
 
         sync_client = Client.new(api_key: API_KEY, sync_mode: true, on_error: on_error)
-        sync_client.capture(Queued::CAPTURE)
+        expect(sync_client.capture(Queued::CAPTURE)).to be(false)
 
         expect(error_status).to eq(400)
       end
 
+      it 'allows an exception to be retried after a failed synchronous request' do
+        PostHog::Transport.stub = false
+        request = stub_request(:post, 'https://us.i.posthog.com/batch/').to_return(status: 400).then
+                                                                        .to_return(status: 200)
+        on_error = proc {}
+        expect(on_error).to receive(:call).with(400, anything).once
+        sync_client = Client.new(api_key: API_KEY, sync_mode: true, on_error: on_error)
+        error = StandardError.new('boom')
+
+        expect(sync_client.capture_exception(error)).to be(false)
+        expect(sync_client.capture_exception(error)).to be(true)
+        expect(sync_client.capture_exception(error)).to be(false)
+        expect(request).to have_been_requested.twice
+      end
+
+      it 'allows an exception to be retried after an oversized event was dropped' do
+        sync_client = Client.new(api_key: API_KEY, sync_mode: true)
+        error = StandardError.new('boom')
+        properties = { details: 'x' * PostHog::Defaults::Message::MAX_BYTES }
+
+        expect(sync_client.capture_exception(error, nil, properties)).to be(false)
+        expect(sync_client.capture_exception(error)).to be(true)
+        expect(sync_client.capture_exception(error)).to be(false)
+      end
+
       [201, 202, 204].each do |status|
         it "does not call on_error for a #{status} response" do
           on_error = proc {}
@@ -289,7 +314,7 @@ def shutdown
           )
 
           sync_client = Client.new(api_key: API_KEY, sync_mode: true, on_error: on_error)
-          sync_client.capture(Queued::CAPTURE)
+          expect(sync_client.capture(Queued::CAPTURE)).to be(true)
         end
       end
 
@@ -327,7 +352,7 @@ def shutdown
         )
 
         sync_client = Client.new(api_key: API_KEY, sync_mode: true, on_error: on_error)
-        sync_client.capture(Queued::CAPTURE)
+        expect(sync_client.capture(Queued::CAPTURE)).to be(false)
 
         expect(error_status).to eq(-1)
         expect(error_message).to include('Serialization error')
@@ -2139,6 +2164,7 @@ def release
           raise StandardError, 'Test exception'
         rescue StandardError => e
           client.capture_exception(e, 'user-123', { 'user_agent' => 'Test Agent', 'request_id' => 'req-123' })
+          expect(client.capture_exception(e)).to be(false)
         end
 
         message = client.dequeue_last_message
@@ -2162,6 +2188,54 @@ def release
         expect(client.dequeue_last_message[:timestamp]).to eq('2024-07-16T04:00:00.123Z')
       end
 
+      it 'does not mark an exception when it could not be queued' do
+        error = StandardError.new('boom')
+        allow(client).to receive(:capture).and_return(false, true)
+
+        expect(client.capture_exception(error)).to be(false)
+        expect(client.capture_exception(error)).to be(true)
+        expect(client.capture_exception(error)).to be(false)
+      end
+
+      %i[dup clone].each do |operation|
+        it "captures a distinct exception created with #{operation}" do
+          error = StandardError.new('boom')
+          expect(client.capture_exception(error)).to be(true)
+
+          copy = error.public_send(operation)
+          expect(client.capture_exception(copy)).to be(true)
+          expect(client.capture_exception(copy)).to be(false)
+          expect(client.queued_messages).to eq(2)
+        end
+      end
+
+      it 'captures a replacement exception raised with a different message' do
+        error = StandardError.new('original')
+        client.capture_exception(error)
+
+        expect { raise error, 'different failure' }.to raise_error('different failure') do |replacement|
+          expect(client.capture_exception(replacement)).to be(true)
+        end
+
+        expect(client.queued_messages).to eq(2)
+        expect(client.dequeue_last_message[:properties]['$exception_list'].first['value']).to eq('original')
+        expect(client.dequeue_last_message[:properties]['$exception_list'].first['value']).to eq('different failure')
+      end
+
+      %i[instance_variable_get instance_variable_set].each do |operation|
+        it "still captures when the exception marker's #{operation} fails" do
+          error = StandardError.new('boom')
+          allow(error).to receive(operation).and_raise(StandardError, 'marker failed')
+
+          expect(client.capture_exception(error)).to be(true)
+          expect(client.queued_messages).to eq(1)
+        end
+      end
+
+      it 'can capture frozen exceptions' do
+        expect(client.capture_exception(StandardError.new('boom').freeze)).to be(true)
+      end
+
       it 'captures the full cause chain outermost-first' do
         begin
           begin
@@ -2171,6 +2245,7 @@ def release
           end
         rescue StandardError => e
           client.capture_exception(e, 'user-123')
+          expect(client.capture_exception(e.cause)).to be(false)
         end
 
         message = client.dequeue_last_message