Repository navigation
feat(rails): report Active Job retry failures - #336
Merged
Merged
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-10T08:49:27.823623+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 8ms |
| Format Validation.Event Has Uuid | ✅ | 5ms |
| Format Validation.Event Has Lib Properties | ✅ | 5ms |
| Format Validation.Distinct Id Is String | ✅ | 7ms |
| Format Validation.Token Is Present | ✅ | 6ms |
| Format Validation.Custom Properties Preserved | ✅ | 6ms |
| Format Validation.Event Has Timestamp | ✅ | 7ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 5ms |
| Retry Behavior.Retries On 503 | ✅ | 5288ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 6ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 4ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8012ms |
| Retry Behavior.Implements Backoff | ❌ | 337ms |
| Retry Behavior.Retries On 500 | ✅ | 5152ms |
| Retry Behavior.Retries On 502 | ✅ | 5116ms |
| Retry Behavior.Retries On 504 | ✅ | 5114ms |
| Retry Behavior.Max Retries Respected | ❌ | 676ms |
| Deduplication.Generates Unique Uuids | ✅ | 15ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5142ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10346ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5119ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 15ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 8ms |
| Compression.Sends Gzip When Enabled | ✅ | 5ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 4ms |
| 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 | ✅ | 5154ms |
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 | ✅ | 8ms |
| 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 | ✅ | 6ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 6ms |
| Request Payload.Groups Round Trip | ✅ | 6ms |
| Request Payload.Groups Default To Empty Object | ✅ | 5ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 6ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 5ms |
| 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 | ✅ | 8ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 5ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 108ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 137ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 8ms |
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-10T08:49:53.319658+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 106ms |
| Format Validation.Event Has Uuid | ✅ | 103ms |
| Format Validation.Event Has Lib Properties | ✅ | 105ms |
| Format Validation.Distinct Id Is String | ✅ | 104ms |
| Format Validation.Token Is Present | ✅ | 104ms |
| Format Validation.Custom Properties Preserved | ✅ | 104ms |
| Format Validation.Event Has Timestamp | ✅ | 104ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 4ms |
| Retry Behavior.Retries On 503 | ✅ | 5407ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2105ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2106ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8106ms |
| Retry Behavior.Implements Backoff | ✅ | 15719ms |
| Retry Behavior.Retries On 500 | ✅ | 5210ms |
| Retry Behavior.Retries On 502 | ✅ | 5209ms |
| Retry Behavior.Retries On 504 | ✅ | 5209ms |
| Retry Behavior.Max Retries Respected | ✅ | 15609ms |
| Deduplication.Generates Unique Uuids | ✅ | 107ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5208ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10407ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5210ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 107ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 104ms |
| Compression.Sends Gzip When Enabled | ✅ | 103ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 104ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 3ms |
| Batch Format.Multiple Events Batched Together | ✅ | 107ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2106ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2105ms |
| Error Handling.Retries On 408 | ✅ | 5208ms |
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 104ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 103ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 104ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 103ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 104ms |
| Request Payload.Groups Round Trip | ✅ | 103ms |
| Request Payload.Groups Default To Empty Object | ✅ | 104ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 104ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 103ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 104ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 103ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 107ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 104ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 204ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 236ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 105ms |
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 |
marandaneto
marked this pull request as ready for review
November 6, 2026 14:50
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
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.
dustinbyrne
reviewed
Nov 6, 2026
dustinbyrne
left a comment
dustinbyrne
left a comment
Contributor
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.
Member
Author
|
Both repeated review concerns are addressed in 2f0becd, pushed with signed main merge e3f1a49. Retry-only exclusion regressions went from 5 failures to green, and the real matched Active Job 5.2.8.1 lifecycle went from missing the opted-in retry report to passing default-off, scheduling, terminal, identity/context and handler controls. All four inline threads now have specific evidence and are resolved. Base and optional Rails 7.2.3.1 suites pass with 2198 examples; Rails 8.1.4 passes all 2198. Lint, API check and exact-HEAD isolated autoreview pass. The PR description records the standalone 5.2 harness limits and unchanged terminal behavior. All nine jobs in the pushed Unit Tests workflow passed: https://github.com/PostHog/posthog-ruby/actions/runs/42346959091. |
5 of 6 tasks
ioannisj
approved these changes
Nov 9, 2026
ioannisj
left a comment
ioannisj
left a comment
Contributor
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
One question on the handled value for retried failures, otherwise nothing from me. Went through the Rails 6+ notification path, the 5.2 interception, the exclusion chain walk, identity and argument handling, the new specs, CI and the changeset.
…reporting-zPpC27 # Conflicts: # public_api_snapshot.txt
Member
Author
|
Merged the latest Final local base and optional OpenTelemetry suites each pass 2,389 examples with no failures. Rails 5.2 lifecycle, lint, public API and exact-head isolated branch autoreview pass. All 30 checks passed for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 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 can now opt in to reporting those failures when Active Job schedules a retry. Rails 6+ uses its retry notification. Rails 5.2 retains the error during
rescue_with_handlerand observesretry_jobwithout replacing the handler or scheduling logic.Retry reports use the job's effective saved or extracted identity and session, filtered arguments, and existing core exception-object deduplication. Request and worker properties stay isolated. Missing or disabled clients do nothing. Reporting failures do not interrupt retries. Discarded failures and failures consumed by rescue or terminal retry handlers do not gain reports.
Retry reports honor default and configured
excluded_exceptionsacross the serialized cause chain, including cycle detection and the 50-entry limit. Excluded failures still schedule retries. Existing terminal capture behavior and metadata are unchanged.New retry events use source
rails.active_job, levelerror, and mechanismtaskwithhandled: true. The relevant boundary is framework job execution, including the retry handler: Active Job consumes the failure and reschedules instead of allowing it to escape. Terminal failures that escape job execution remain unhandled. The cross-SDK boundary clarification is recorded in sdk-specs#205, which is awaiting review. Source and level use the existing additional-properties path rather than a typed internal metadata channel. Retry filtering follows the capture-exception ignored-type chain rule while retaining the existing Rails default exclusion list rather than introducing a new empty-by-default list.A Rails-only minor changeset, initializer examples, README guidance, and intentional public API snapshot additions are included. The existing changeset was updated rather than duplicated. The branch includes a signed merge of
origin/mainatceb36ce01e7eb0d090102e02dcd3afff89279e0c, committed as9f85498ebe7bccb67cda7df7a7c1fdac7bcec684, without rewriting history. The earlier signed mergea0998b8aa777483e3d7d03028b52d5bac6edba6cremains in history. Its only conflict was inpublic_api_snapshot.txt. The latest merge also includes the controller exception helper that landed afterward and had no conflicts. Regenerating it retained both the notification logs API from main and this PR’s retry option.💚 How did you test it?
Local validation of merged head
9f85498ebe7bccb67cda7df7a7c1fdac7bcec684used Ruby 4.0.7, Bundler 4.0.13 and Rails 7.2.3.1.bundle exec rspec: 2389 examples, 0 failures, 43 optional-integration pending.BUNDLE_GEMFILE=gemfiles/otel.gemfile BUNDLE_PATH=/Users/marandaneto/Github/posthog-ruby/vendor/bundle bundle exec rspec: 2389 examples, 0 failures, 2 existing MCP pending.bundle exec rubocop: 163 files, no offenses.bundle exec rake public_api:generateresolved the snapshot conflict.bundle exec rake public_api:check,git diff --check, and the unmerged-file check passed.origin/mainand reported no actionable findings. Diff-scoped cleanup and the Authoring-mode test audit found no blockers.Review concerns were reproduced before their production fixes. Direct and nested configured/default exclusions and the entry-50 boundary failed with unexpected reports, then passed after filtering. The Rails 5.2 lifecycle failed with
expected 1 retry events, got 0, then passed after compatibility support. For the handled-state follow-up, the retry suite failed four examples because retry events still hadhandled: false, and the real Rails 5.2 lifecycle failed withretry not marked handled. Both passed after the metadata fix. Retained assertions verify that escaping terminal failures remain unhandled.Tests retain scheduling, original exceptions and returns, single handler invocation, identity/context isolation, exclusion boundaries and cycles, stale-error cleanup, client gates, privacy hooks, deduplication, eventual success, reporting failures, and both Rails initialization load orders. The complete historical Ruby/Rails matrix was not run locally. A Rails 8.1.4 full-suite run passed before the handled-state follow-up, but was not repeated locally for this merge. All 30 GitHub checks passed for exact merged head
9f85498ebe7bccb67cda7df7a7c1fdac7bcec684, including Unit Tests, latest Rails, optional integrations, API, lint, builds, compliance, security and benchmarks. Human review is still required.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe changeset is hand-authored. I did not run
pnpm changesetor add a duplicate.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
marandaneto is the human DRI and PR assignee. Pi's
workerused shell/file tools, Git, GitHub CLI, and theautoreviewhelper with theclaude-code/claude-opus-5-5provider. There is no public session URL.The approved follow-up scope adds retry-only exclusion filtering, supported Active Job 5.2 compatibility, and handled metadata for framework-consumed retries. The handled-state choice and separate spec clarification were explicitly approved by marandaneto. It leaves terminal exclusions, tracing features, and other PRs untouched. The branch was updated using a new signed merge, without rewriting history. This PR will not be self-approved or merged.