Repository navigation
feat(rails): report Active Job retry failures - #336
marandaneto wants to merge 1 commit into
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-06T14:36:52.638564+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 8ms |
| Format Validation.Event Has Uuid | ✅ | 6ms |
| Format Validation.Event Has Lib Properties | ✅ | 4ms |
| Format Validation.Distinct Id Is String | ✅ | 8ms |
| Format Validation.Token Is Present | ✅ | 7ms |
| Format Validation.Custom Properties Preserved | ✅ | 5ms |
| Format Validation.Event Has Timestamp | ✅ | 7ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 6ms |
| Retry Behavior.Retries On 503 | ✅ | 5234ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 5ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 6ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8011ms |
| Retry Behavior.Implements Backoff | ❌ | 238ms |
| Retry Behavior.Retries On 500 | ✅ | 5112ms |
| Retry Behavior.Retries On 502 | ✅ | 5141ms |
| Retry Behavior.Retries On 504 | ✅ | 5146ms |
| Retry Behavior.Max Retries Respected | ❌ | 595ms |
| Deduplication.Generates Unique Uuids | ✅ | 13ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5113ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10300ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5152ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 15ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 8ms |
| Compression.Sends Gzip When Enabled | ✅ | 6ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 5ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 3ms |
| Batch Format.Multiple Events Batched Together | ❌ | 13ms |
| Error Handling.Does Not Retry On 403 | ❌ | 4ms |
| Error Handling.Does Not Retry On 413 | ❌ | 4ms |
| Error Handling.Retries On 408 | ✅ | 5111ms |
Failures
retry_behavior.does_not_retry_on_400
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
retry_behavior.does_not_retry_on_401
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
retry_behavior.implements_backoff
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
retry_behavior.max_retries_respected
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
batch_format.multiple_events_batched_together
Expected 1 requests, got 5
error_handling.does_not_retry_on_403
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
error_handling.does_not_retry_on_413
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 9ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 7ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 7ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 7ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 5ms |
| Request Payload.Groups Round Trip | ✅ | 5ms |
| Request Payload.Groups Default To Empty Object | ✅ | 5ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 5ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 7ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 5ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 5ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 7ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 5ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 141ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 146ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 6ms |
Failures
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']
posthog-ruby-async Compliance ReportDate: 2026-11-06T14:37:36.118922+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 108ms |
| Format Validation.Event Has Uuid | ✅ | 105ms |
| Format Validation.Event Has Lib Properties | ✅ | 107ms |
| Format Validation.Distinct Id Is String | ✅ | 105ms |
| Format Validation.Token Is Present | ✅ | 106ms |
| Format Validation.Custom Properties Preserved | ✅ | 106ms |
| Format Validation.Event Has Timestamp | ✅ | 106ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 107ms |
| Retry Behavior.Retries On 503 | ✅ | 5412ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2107ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2108ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8012ms |
| Retry Behavior.Implements Backoff | ✅ | 15611ms |
| Retry Behavior.Retries On 500 | ✅ | 5211ms |
| Retry Behavior.Retries On 502 | ✅ | 5210ms |
| Retry Behavior.Retries On 504 | ✅ | 5211ms |
| Retry Behavior.Max Retries Respected | ✅ | 15508ms |
| Deduplication.Generates Unique Uuids | ✅ | 109ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5210ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10311ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5213ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 109ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 106ms |
| Compression.Sends Gzip When Enabled | ✅ | 105ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 105ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 4ms |
| Batch Format.Multiple Events Batched Together | ✅ | 109ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2108ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2109ms |
| Error Handling.Retries On 408 | ✅ | 5211ms |
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 106ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 106ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 106ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 106ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 105ms |
| Request Payload.Groups Round Trip | ✅ | 107ms |
| Request Payload.Groups Default To Empty Object | ✅ | 106ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 106ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 106ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 106ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 104ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 109ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 105ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 253ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 208ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 108ms |
Failures
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']
SDK performance: posthog-rubyBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed b9ed274: 2 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
| base.around_perform :with_posthog_job_context, prepend: true if base.respond_to?(:around_perform) | ||
| return if @retry_subscription | ||
|
|
||
| @retry_subscription = ActiveSupport::Notifications.subscribe('enqueue_retry.active_job') do |*args| |
There was a problem hiding this comment.
P2 · Support retry reporting on Rails 5.2
On Rails 5.2, which the gemspec explicitly supports, retry_job calls enqueue without emitting enqueue_retry.active_job; that notification was introduced in Rails 6. Consequently, enabling both configuration options silently captures no retry failures on Rails 5.2. Add a compatible retry interception path for that supported version.
| return unless config&.auto_instrument_active_job && config.report_active_job_retries && exception | ||
| return unless PostHog.initialized? && PostHog.client.enabled? |
There was a problem hiding this comment.
P2 · Honor exception exclusions in the new retry capture path
With retry reporting enabled, an exception listed in excluded_exceptions still reaches capture_job_exception and uploads its message, stack and arguments. This new capture path violates the capture-exception spec's requirement to apply ignored types across every reporting path. Filter excluded types, including chained causes, before capture.
dustinbyrne
left a comment
dustinbyrne
left a comment
There was a problem hiding this comment.
Two issues remain in the new retry-reporting path: configured exception exclusions are ignored, and Rails 5.2 never emits the subscribed notification. Consider honoring exclusions and either supporting that runtime or documenting an explicit capability restriction.
AI-assisted review.
| return unless config&.auto_instrument_active_job && config.report_active_job_retries && exception | ||
| return unless PostHog.initialized? && PostHog.client.enabled? |
There was a problem hiding this comment.
[P1 / IMPORTANT] Honor exclusions before capturing the new retry report
Relationship: New-path exposure/amplification of an existing job-exclusion bypass.
Location: posthog-rails/lib/posthog/rails/active_job.rb:155–161.
When retry reporting is enabled, a job raising a configured excluded exception—or wrapping an excluded cause—still queues an exception event. It should schedule the retry without admitting that report to capture.
PostHog::Rails.configure do |config|
config.auto_instrument_active_job = true
config.report_active_job_retries = true
config.excluded_exceptions = ['ArgumentError']
end
class RetryingJob < ApplicationJob
retry_on StandardError, wait: 0, attempts: 2
def perform(nested = false)
raise ArgumentError, 'excluded inner' unless nested
begin
raise ArgumentError, 'excluded inner'
rescue ArgumentError
raise RuntimeError, 'outer'
end
end
endWith the normal initialized Rails integration, both RetryingJob.perform_now(false) and RetryingJob.perform_now(true) currently produce a retry report on Rails 7.1.6.
The new helper checks configuration and client availability but never checks exclusions; neither its shared helper nor core capture_exception adds a later check. The pinned ignored-type contract requires filtering on every reporting path and across the serialized chain. The added test at active_job_retries_spec.rb:185–190 instead enshrines bypass.
Executed regression: Ruby 3.4.7 / Rails 7.1.6, real facade/extension and test-mode client. Two cases configure excluded_exceptions = ['ArgumentError'], run the direct or nested failure through public perform_now, assert one scheduled retry, then assert zero queued reports. An ordinary unexcluded retry is the positive control. Base: 3 examples / 0 failures, exit 0. Head: 3 examples / 2 failures, exit 1; both excluded cases get 1, expected 0. The positive control passes.
The base has no retry-reporting option: this proves newly exposed reporting, not prior support for retry filtering. Terminal capture already bypasses exclusions on both revisions and is outside this finding. Queueing—not actual external transmission—was demonstrated. The impact is unwanted event volume and admission of exception messages, stacks, and permitted job properties into normal delivery; parameter filtering and before_send are not bypassed.
Consider filtering only the new retry path before capture, checking the same cause chain the serializer retains with its cycle detection and 50-entry bound. Calling the current outer-only predicate once would miss the demonstrated cause case. Retain both regression cases and the unexcluded control. If divergence is accepted instead, document its consequences in public configuration guidance; PR disclosure alone does not establish that acceptance.
| base.around_perform :with_posthog_job_context, prepend: true if base.respond_to?(:around_perform) | ||
| return if @retry_subscription | ||
|
|
||
| @retry_subscription = ActiveSupport::Notifications.subscribe('enqueue_retry.active_job') do |*args| |
There was a problem hiding this comment.
[P1 / IMPORTANT] Avoid silently unsupported retry reporting on Rails 5.2
Relationship: Introduced capability/support gap, not a regression in default or terminal behavior.
Location: posthog-rails/lib/posthog/rails/active_job.rb:18–21.
A Rails 5.2 application can enable both options and successfully schedule an ordinary retry, but receives no retry report:
config.auto_instrument_active_job = true
config.report_active_job_retries = true
class RetryVisibilityJob < ApplicationJob
retry_on RuntimeError, wait: 0, attempts: 2
def perform
raise 'temporary failure'
end
end
RetryVisibilityJob.perform_nowThe gem explicitly supports Railties >= 5.2.0; the new README and templates state no capability restriction. However, Active Job 5.2.8.1's retry_job directly calls enqueue without emitting enqueue_retry.active_job. Its retry handler also does not pass the error. Official 6.0.6.1 and 7.1.6 sources do emit the notification this subscriber requires.
Executed lifecycle comparison: Ruby 3.4.7, matched real Active Job/ActiveSupport packages, real extension/facade and test-mode client:
| Snapshot | Active Job | Retry reports | Terminal reports | Scheduled retries |
|---|---|---|---|---|
| Base | 5.2.8.1 | 0 | 1 | 1 |
| Head | 5.2.8.1 | 0 | 1 | 1 |
| Base | 7.1.6 | 0 | 1 | 1 |
| Head | 7.1.6 | 1 | 1 | 1 |
The head 5.2 assertion fails with Expected 1 retry events, got 0, exit 1. The base prior-behavior control expects zero because this option does not exist there, and exits 0. Thus this is not a same-API regression claim. The harness executes public perform_now, JSON-round-tripped serialize, and ActiveJob::Base.execute; no retry implementation was patched.
The 5.2 probe is standalone Active Job with small Rails version/application/config-accessor shims, not a fully booted Rails 5.2 application. Official source absence, successful scheduling, and working terminal capture independently isolate the missing notification. The impact is limited to missing opted-in observability, not broken job execution.
Consider either adding compatible older-framework reporting, preserving scheduling/error behavior and newer-version deduplication, or explicitly restricting and documenting the capability's Rails range in the README and both templates. A documented, accepted restriction is a valid resolution; raising the whole gem's minimum Rails version is unnecessary. Add a lowest-supported-capability regression with default-off, scheduling, and terminal controls.
💡 Motivation and Context
Public API addition:
PostHog::Rails::Configuration#report_active_job_retries, defaultfalse. It requiresauto_instrument_active_job.Active Job can retry a failed execution without letting the error escape to the terminal exception handler. Applications that need to investigate these failures can now opt in to reporting them when Active Job schedules a retry.
The notification subscriber uses the job's effective saved or extracted identity and session. Retry events reuse filtered job arguments and omit request and worker properties. Missing or disabled clients do nothing. Reporting errors do not interrupt retries. Discarded failures and failures consumed by rescue or terminal retry handlers do not gain reports. Core exception-object deduplication still applies, so reusing one exception object can suppress later retry or terminal reports.
New retry events use source
rails.active_job, levelerror, and mechanismtaskwithhandled: falseat the job execution boundary, following the exception-event-metadata spec. The existing additional-properties path carries source and level rather than a typed internal metadata channel. The existing job exclusion behavior is unchanged:excluded_exceptionsdoes not filter job reports. This differs from the capture-exception spec's cross-path ignored-type rule. Terminal event metadata is unchanged.A Rails-only minor changeset, initializer examples, README guidance, and the intentional public API snapshot update are included. No package versions are changed by hand.
💚 How did you test it?
Local validation used Ruby 4.0.7, Bundler 4.0.13, and Rails 7.2.3.1.
bundle exec rspec: 2174 examples, 0 failures, 40 pending because optional integrations are unavailable in the base bundle.BUNDLE_PATH=/Users/marandaneto/Github/posthog-ruby/vendor/bundle BUNDLE_GEMFILE=gemfiles/otel.gemfile bundle exec rspec: 2174 examples, 0 failures, 2 existing pending.bundle exec rubocop: 151 files inspected, no offenses.bundle exec rake public_api:checkandgit diff --check: passed.Tests execute real Active Job retries through serialized queue payloads and inspect captured exception events. They cover saved, extracted and numeric identities, nested application context, nested argument filtering, worker context restoration, eventual success, reporting failures, deduplication, exclusions, missing clients, and handled failure boundaries. Two subprocess tests initialize real Rails applications with Active Job loaded before or after the load hook. The Authoring-mode test audit retained all cases. The historical Ruby/Rails matrix was not run locally.
The required isolated autoreview reviewed the exact signed HEAD against
origin/mainand returned no findings. Human review is still required.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe changeset file is included and was hand-authored. I did not run
pnpm changesetor generate a duplicate.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
marandaneto is the human DRI and PR assignee. Pi's
workersubagent used shell/file tools, Git, GitHub CLI, and theautoreviewhelper with theclaude-code/claude-opus-5-5provider. The local Pi session ID is01a1b0e4-9039-71a4-a297-fed90e502ce9. There is no public session URL.The directed scope keeps reporting opt-in, preserves terminal behavior and job exclusions, and restores effective job identity rather than reading a nested application scope. Diff-scoped cleanup and the test audit ran before the signed commit. This draft needs human review and will not be self-approved or merged.