Repository navigation
feat(rails): capture terminating runner exceptions - #341
Open
marandaneto wants to merge 5 commits into
Open
marandaneto wants to merge 5 commits into
marandaneto wants to merge 5 commits into
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-07T07:04:32.689414+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 9ms |
| Format Validation.Event Has Uuid | ✅ | 5ms |
| Format Validation.Event Has Lib Properties | ✅ | 5ms |
| Format Validation.Distinct Id Is String | ✅ | 8ms |
| Format Validation.Token Is Present | ✅ | 7ms |
| Format Validation.Custom Properties Preserved | ✅ | 6ms |
| Format Validation.Event Has Timestamp | ✅ | 7ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 6ms |
| Retry Behavior.Retries On 503 | ✅ | 5254ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 6ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 6ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8011ms |
| Retry Behavior.Implements Backoff | ❌ | 340ms |
| Retry Behavior.Retries On 500 | ✅ | 5156ms |
| Retry Behavior.Retries On 502 | ✅ | 5116ms |
| Retry Behavior.Retries On 504 | ✅ | 5148ms |
| Retry Behavior.Max Retries Respected | ❌ | 607ms |
| Deduplication.Generates Unique Uuids | ✅ | 14ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5113ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10324ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5120ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 16ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 8ms |
| Compression.Sends Gzip When Enabled | ✅ | 5ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 5ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 3ms |
| Batch Format.Multiple Events Batched Together | ❌ | 14ms |
| Error Handling.Does Not Retry On 403 | ❌ | 4ms |
| Error Handling.Does Not Retry On 413 | ❌ | 4ms |
| Error Handling.Retries On 408 | ✅ | 5155ms |
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 | ✅ | 8ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 8ms |
| 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 | ✅ | 6ms |
| 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 | ✅ | 6ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 138ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 110ms |
| 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-07T07:04:54.178509+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 9ms |
| Format Validation.Event Has Uuid | ✅ | 105ms |
| Format Validation.Event Has Lib Properties | ✅ | 108ms |
| Format Validation.Distinct Id Is String | ✅ | 106ms |
| Format Validation.Token Is Present | ✅ | 106ms |
| Format Validation.Custom Properties Preserved | ✅ | 108ms |
| Format Validation.Event Has Timestamp | ✅ | 106ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 8ms |
| Retry Behavior.Retries On 503 | ✅ | 5311ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2193ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2109ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8116ms |
| Retry Behavior.Implements Backoff | ✅ | 15614ms |
| Retry Behavior.Retries On 500 | ✅ | 5209ms |
| Retry Behavior.Retries On 502 | ✅ | 5210ms |
| Retry Behavior.Retries On 504 | ✅ | 5213ms |
| Retry Behavior.Max Retries Respected | ✅ | 15724ms |
| Deduplication.Generates Unique Uuids | ✅ | 111ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5211ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10311ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5214ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 112ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 107ms |
| Compression.Sends Gzip When Enabled | ✅ | 106ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 106ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 4ms |
| Batch Format.Multiple Events Batched Together | ✅ | 112ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2109ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2109ms |
| Error Handling.Retries On 408 | ✅ | 5212ms |
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 108ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 107ms |
| 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 | ✅ | 107ms |
| Request Payload.Groups Round Trip | ✅ | 107ms |
| Request Payload.Groups Default To Empty Object | ✅ | 107ms |
| 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 | ✅ | 107ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 3ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 105ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 111ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 108ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 212ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 247ms |
| 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 |
marandaneto
marked this pull request as ready for review
November 6, 2026 14:52
![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 6cd4090: 1 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.
The ordinary runner paths and shutdown ordering look sound. Two regressions remain in the deferred handoff: inline Rails-converted errors bypass exclusions/deduplication, and runner reports lose existing user/context enrichment. Both reproduce with equivalent base/head tests on Rails 7.2.4 and 8.1.4. Details and regression cases are inline.
AI-assisted review.
Member
Author
|
Both deferred-runner review concerns are addressed in signed commit 73195fa, now pushed at 9c55f75 after merging pinned main. Real Rails 7.2.3.1 and 8.1.4 subprocess/HTTP regressions failed before the fix and now pass. The original inline error is retained for exclusions/deduplication, and existing reporter identity/context survives deferral without applying stale context to a later failure. Full base, optional and latest-Rails suites passed locally. All nine Unit Tests jobs for the pushed SHA passed: https://github.com/PostHog/posthog-ruby/actions/runs/42347049445. The PR description contains exact counts and compatibility limits. |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
This branch has not been deployed
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
Terminating
rails runnerfailures need to reach PostHog before the SDK shuts down. Executor reports can otherwise capture errors before the terminal boundary with non-terminal metadata, including successful exits.This adds a runner marker and captures the terminal exception in the existing exit hook before the queue drains. Events use source
rails.runner, levelfatal, and anat_exitmechanism marked unhandled and non-synthetic. Normal completion and successful exits are ignored. Original exit status and error output are preserved, including when reporting fails or a later exit hook changes the terminal exception.Unhandled runner executor reports retain the original exception object, reporter-resolved identity and safely serialized context until exit. When Rails converts an inline
NameErrororSyntaxErrorintoexit 1, the handoff uses the original object for exclusions and deduplication. This also works with--skip-executor, where Rails emits no reporter event. It matches the cause type, status and actual Rails command caller rather than unwrapping arbitrary applicationSystemExitobjects. Inline eval shares Rails' source filename, so filename matching alone is not used as proof of conversion. A different terminal exception does not inherit stale reporter context. The deferred record is consumed and cleared before terminal capture gates run.auto_capture_exceptionsremains opt-in and defaults to false. Only initialized, enabled clients are used. Exclusions,before_send, normal transport and existing client deduplication still apply. An exception already captured through the client keeps its earlier event. No new public option or method is exposed. The existing changeset selects a minor release forposthog-railsonly.Delivery is best-effort. Failures before runner hooks load,
exit!, and termination that bypasses Ruby exit hooks cannot be captured. No separate runner-code, command-line argument or request-body properties are added. Existing reporter fields are preserved, not expanded. Exception messages and stack source context still follow the normal exception pipeline and may contain application data.The touched behavior follows the pinned capture-exception, exception-event-metadata and before-send-hook specifications. No new divergence was identified. Unrelated existing conformance gaps are unchanged.
💚 How did you test it?
Local validation used Ruby 4.0.7 and Bundler 4.0.13. Full suites passed on Rails 7.2.3.1 and Rails 8.1.4 with latest cooldown-eligible dependencies resolved from this worktree's optional bundle. The historical Ruby/Rails matrix was not run locally.
The 38 runner examples boot real temporary Rails applications in subprocesses and inspect loopback queued HTTP delivery, exit status and stderr. They cover file and inline failures, original exception metadata, exclusions, manual-capture deduplication with and without executor reporting, reporter
user_idanddistinct_id, circular context serialization, context scrubbing, arbitrarySystemExitcauses, later exit-hook errors and exits, successful exits, disabled capture/client, uninitialized clients, non-runner processes and reporting failures. File and inline cases compare error output byte-for-byte against disabled capture.Before the fixes, eight added assertions failed on the reviewed head on Rails 7.2.3.1 and 8.1.4 for the reported exclusions, deduplication, original-object metadata and reporter-context regressions. A later controlled pre-fix replay of the then-current runner suite on Rails 8.1.4 produced nine expected failures, including the retained-identity assertion in the context-scrubbing case. Restoring the fix made all runner cases pass. A follow-up reproduced the same exclusion and duplicate-capture failures under
--skip-executor: four failures among six new cases on both Rails versions before the fix, then all six passed. The two controls preserve intentional application exits 1 and 23 with a NameError cause. Dependency/setup failures were not counted as reproductions.Diff-scoped cleanup and Authoring-mode test audit covered the complete runner suite and shared helper, with independent oracles from received HTTP events and Ruby process output/status. Exact final HEAD
43501a574edf9b2345a2a3525c3dc46c5ee68599passed isolated autoreview with no findings. The fixes and pinned-main merge are signed and locally verified. GitHub also verifies the current follow-up commit signature as valid.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe existing hand-authored feature changeset was updated. No duplicate changeset or version bump was generated.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
marandaneto directed the work and is the human DRI. Pi's
workerimplementation subagent used file read/edit/write tools, bash, Git and GitHub CLI. The autoreview helper ran an isolated Pi review withclaude-code/claude-opus-5-5. There is no public session URL.The implementation keeps the existing exit hook and capture pipeline rather than adding a transport or public setting. Review feedback was reproduced with real Rails subprocesses before fixing the deferred handoff. This PR requires human review. No self-approval, merge or auto-merge is requested.