Repository navigation
feat(rails): capture terminating runner exceptions - #341
marandaneto wants to merge 2 commits into
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-06T14:43:46.682998+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 11ms |
| Format Validation.Event Has Uuid | ✅ | 7ms |
| Format Validation.Event Has Lib Properties | ✅ | 7ms |
| Format Validation.Distinct Id Is String | ✅ | 9ms |
| Format Validation.Token Is Present | ✅ | 8ms |
| Format Validation.Custom Properties Preserved | ✅ | 9ms |
| Format Validation.Event Has Timestamp | ✅ | 8ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 7ms |
| Retry Behavior.Retries On 503 | ✅ | 5285ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 7ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 6ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8018ms |
| Retry Behavior.Implements Backoff | ❌ | 261ms |
| Retry Behavior.Retries On 500 | ✅ | 5155ms |
| Retry Behavior.Retries On 502 | ✅ | 5156ms |
| Retry Behavior.Retries On 504 | ✅ | 5149ms |
| Retry Behavior.Max Retries Respected | ❌ | 455ms |
| Deduplication.Generates Unique Uuids | ✅ | 17ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5113ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10242ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5121ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 19ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 9ms |
| Compression.Sends Gzip When Enabled | ✅ | 7ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 6ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 3ms |
| Batch Format.Multiple Events Batched Together | ❌ | 17ms |
| Error Handling.Does Not Retry On 403 | ❌ | 5ms |
| Error Handling.Does Not Retry On 413 | ❌ | 5ms |
| Error Handling.Retries On 408 | ✅ | 5147ms |
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 | ✅ | 10ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 8ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 9ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 7ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 7ms |
| Request Payload.Groups Round Trip | ✅ | 7ms |
| Request Payload.Groups Default To Empty Object | ✅ | 6ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 7ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 7ms |
| 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 | ✅ | 7ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 9ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 7ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 135ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 110ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 10ms |
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:44:25.463049+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 10ms |
| Format Validation.Event Has Uuid | ✅ | 105ms |
| Format Validation.Event Has Lib Properties | ✅ | 109ms |
| Format Validation.Distinct Id Is String | ✅ | 107ms |
| Format Validation.Token Is Present | ✅ | 106ms |
| Format Validation.Custom Properties Preserved | ✅ | 108ms |
| Format Validation.Event Has Timestamp | ✅ | 107ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 7ms |
| Retry Behavior.Retries On 503 | ✅ | 5310ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2109ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2108ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8116ms |
| Retry Behavior.Implements Backoff | ✅ | 15623ms |
| Retry Behavior.Retries On 500 | ✅ | 5212ms |
| Retry Behavior.Retries On 502 | ✅ | 5208ms |
| Retry Behavior.Retries On 504 | ✅ | 5212ms |
| Retry Behavior.Max Retries Respected | ✅ | 15508ms |
| Deduplication.Generates Unique Uuids | ✅ | 112ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5211ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10418ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5214ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 112ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 108ms |
| Compression.Sends Gzip When Enabled | ✅ | 107ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 106ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 5ms |
| Batch Format.Multiple Events Batched Together | ✅ | 113ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2108ms |
| 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 | ✅ | 107ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 107ms |
| 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 | ✅ | 107ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 107ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 107ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 4ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 106ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 113ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 106ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 210ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 239ms |
| 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 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.
| return if exception.is_a?(SystemExit) && exception.success? | ||
| return unless PostHog.initialized? && PostHog.client.enabled? | ||
| return unless PostHog::Rails.config&.auto_capture_exceptions | ||
| return unless PostHog::Rails.config.should_capture_exception?(exception) |
There was a problem hiding this comment.
P2 · Preserve the original error when Rails converts it to SystemExit
For inline rails runner code, Rails rescues NameError/SyntaxError and invokes exit 1. After the subscriber drops that report, this boundary captures SystemExit instead: exclusions on the original error no longer apply, and a manually captured error is sent again as a cause. Preserve the original runner exception for filtering and deduplication.
dustinbyrne
left a comment
dustinbyrne
left a comment
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.
| return if exception.is_a?(SystemExit) && exception.success? | ||
| return unless PostHog.initialized? && PostHog.client.enabled? | ||
| return unless PostHog::Rails.config&.auto_capture_exceptions | ||
| return unless PostHog::Rails.config.should_capture_exception?(exception) |
There was a problem hiding this comment.
[P1] Preserve the original runner error before Rails converts it to SystemExit
For inline rails runner code, excluding NameError no longer prevents that error from being sent. Rails rescues the original exception and calls exit 1; this hook then filters and deduplicates the new SystemExit, not the original error. The resulting event still includes the excluded NameError as its cause. An already manually captured inline error likewise produces a second event.
With an initialized, enabled client and auto_capture_exceptions = true, these supported callers reproduce the problem:
# Application initializer
PostHog::Rails.configure { |c| c.excluded_exceptions = ['NameError'] }bin/rails runner "raise NameError, 'excluded inline failure'"The base sends 0 events; this head sends 1 $exception whose list is [SystemExit, NameError]. Without the exclusion, this caller sends 1 event on base but 2 on head:
bin/rails runner "begin; raise NameError, 'already captured inline'; rescue NameError => e; PostHog.capture_exception(e); raise; end"Rails' official RunnerCommand#perform converts inline NameError/SyntaxError into exit 1. The new ErrorSubscriber#report early return discards the original report, then capture_runner_exception checks should_capture_exception? on SystemExit. Client#capture_exception's existing object-based deduplication cannot recognize the new wrapper. This violates the existing excluded_exceptions contract in Configuration#should_capture_exception? and the existing successful-capture deduplication contract (Client#capture_exception, spec/posthog/rails/exception_mechanism_spec.rb). The pinned capture-exception specification also requires existing type-ignore options to consider every exception in the chain. Ruby's broader cause-filtering gap is pre-existing, but this PR exposes it to inline errors that the base correctly excludes. Neither that contract nor fatal/unhandled metadata requires blindly unwrapping every application wrapper.
Executed regression: identical real Rails subprocess/loopback HTTP fixtures on base 58f5335f895ebebc417bb4a48d22351ff96b5fb0 and head 6cd409009a15d9b56c7cb86b13adcaf470055e6c, Ruby 3.4.7, separately Rails 7.2.4 and 8.1.4. Both exclusion and dedup assertions pass on base and fail on head; both processes still exit 1. The PR's 17 existing runner examples pass on head, but all invoke a script file rather than the inline branch. SyntaxError follows the same official Rails rescue path; the executed cases use NameError.
Consider retaining the original executor-reported runner error and using that same object for exclusion checks and capture at the exit boundary when Rails converts it to SystemExit. Preserve intentional nonzero exit reporting and successful-exit suppression; do not indiscriminately unwrap every application SystemExit.
For regression coverage, extend run_application's helper with inline: false and pass (inline ? code : script) as the runner argument. Then add these tests against its existing HTTP receiver:
it 'keeps exclusions for inline runner errors' do
_, _, status = run_application(
"raise NameError, 'excluded inline failure'", inline: true,
setup: "PostHog::Rails.config.excluded_exceptions = ['NameError']"
)
expect(status.exitstatus).to eq(1)
expect(received_events).to eq([])
end
it 'does not recapture an inline error wrapped by Rails' do
_, _, status = run_application(
"begin; raise NameError, 'already captured inline'; rescue NameError => e; " \
"PostHog.capture_exception(e); raise; end", inline: true
)
expect(status.exitstatus).to eq(1)
expect(received_events.length).to eq(1)
endThe full runnable fixture was executed with ruby -Ilib -Iposthog-rails/lib -Ispec -S rspec <external-inline-regression-spec> --example 'inline NameError'; the installed dependency versions were held equal within each base/head pair. Impact is limited to the inline Rails conversion path, not ordinary file-runner RuntimeError filtering or a bypass of before_send itself.
| def report(error, handled:, severity:, context:, source: nil) | ||
| # Runner termination is captured at exit, before the SDK drains its queue. | ||
| # The executor also reports successful SystemExit, which is not a failure. | ||
| return if !handled && source == 'application.runner.railties' |
There was a problem hiding this comment.
[P1] Preserve Rails reporter identity and context when deferring runner errors
A runner that sets Rails.error context now sends its terminal exception under a random identity and loses the diagnostic context. Before this change, the error subscriber used that context to associate the exception with the configured user and attach its context properties.
With an initialized, enabled client and automatic exception capture enabled, an ordinary runner script is enough:
Rails.error.set_context(user_id: 'runner-user', job: 'nightly')
raise 'context failure'On base 58f5335f895ebebc417bb4a48d22351ff96b5fb0, the single received event has distinct_id: 'runner-user' and $context_job: 'nightly'. On head 6cd409009a15d9b56c7cb86b13adcaf470055e6c, there is still one event, but it has a generated UUID distinct ID and no $context_job. The new source rails.runner is expected; discarding the existing identity and diagnostic fields is not necessary to add fatal/unhandled metadata.
The changed return in ErrorSubscriber#report discards the reporter's context. That bypasses the existing context[:user_id] || context[:distinct_id] resolution and $context_* serialization immediately below it. The later Railtie capture passes nil for identity and only the two terminal metadata properties. Rails explicitly documents set_context for error subscribers, including a user_id example (official Rails 7.2.4 source, lines 195–221); this integration already consumes those fields. The existing reporter-context test in spec/posthog/rails/exception_mechanism_spec.rb also asserts identity and diagnostic enrichment. Node's independent deferred autocapture snapshots its original context (PostHog/posthog-js@a0e648577347a0e36c1956fecc66f31623f96f9e, packages/node/src/extensions/error-tracking/autocapture.ts:103–117,314–336); that is supporting parity evidence, not a Rails-specific specification.
Executed regression: a real temporary Rails app invoked through the existing file-runner helper and actual queued HTTP transport, with Ruby 3.4.7 and separately Rails 7.2.4 and 8.1.4. The same test passes on base (1 example, 0 failures) and fails on head (1 example, an aggregate failure for both lost fields) in each pair. Both processes exit 1. No private state or capture mock is needed.
Consider carrying the reporter's resolved identity and safely serialized context alongside the deferred exception, then merging those fields into its terminal capture while retaining the new fatal/unhandled runner metadata. This is independent of recovering the original object from a Rails SystemExit wrapper: this case uses an ordinary file-runner RuntimeError with no conversion.
The following regression uses the existing run_application and received_events helpers without any helper changes:
it 'preserves Rails reporter identity and diagnostic context' do
_, stderr, status = run_application(
"Rails.error.set_context(user_id: 'runner-user', job: 'nightly'); raise 'context failure'"
)
expect(status.exitstatus).to eq(1), stderr
events = received_events
expect(events.length).to eq(1)
aggregate_failures do
expect(events.first['distinct_id']).to eq('runner-user')
expect(events.first.fetch('properties')).to include('$context_job' => 'nightly')
end
endRun with ruby -Ilib -Iposthog-rails/lib -Ispec -S rspec <runner-spec-with-this-example> --example 'preserves Rails reporter identity'; installed dependencies were held equal within each executed base/head pair. This affects runners that supply Rails reporter context; ordinary runners without that context have no prior enrichment to lose.
💡 Motivation and Context
Terminating
rails runnerfailures need to reach PostHog before the SDK shuts down. On newer Rails versions, executor reports can capture the error too early with non-terminal metadata and can even reportexit 0as an exception. Without executor reporting, terminal failures can be missed entirely.This adds a runner-specific marker and captures the terminal exception in the existing exit hook before the queue drains. Unhandled runner executor reports are deferred to that boundary. New events use source
rails.runner, levelfatal, and anat_exitmechanism marked unhandled and non-synthetic. Original exit status and error output are preserved, including when reporting itself fails. Normal completion and successfulSystemExitare ignored.The existing
auto_capture_exceptionssetting remains opt-in and defaults to false. Only initialized, enabled clients are used. Exception exclusions,before_send, normal transport, and duplicate prevention still apply. An exception already captured through the client keeps its earlier event rather than creating a second terminal event. No new public option or method is exposed. The included 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 or command-line argument properties are added. Exception messages and stack source context still follow the normal exception pipeline and may contain application data.The new boundary follows the pinned capture-exception, exception-event-metadata, before-send-hook and shutdown specifications for touched behavior. No new divergence was identified. Existing unrelated metadata and filtering behavior is unchanged.
💚 How did you test it?
Local validation used Ruby 4.0.7, Bundler 4.0.13 and Rails 7.2.3.1. The historical Ruby/Rails matrix was not run locally.
bundle exec rspec: 2169 examples, 0 failures, 40 pending for optional integrations and existing cases.BUNDLE_PATH=/Users/marandaneto/Github/posthog-ruby/vendor/bundle BUNDLE_GEMFILE=gemfiles/otel.gemfile: 2169 examples, 0 failures, 2 existing pending cases.bundle exec rubocop,bundle exec rake public_api:check, and branch whitespace checks passed.The 17 new runner examples boot a real temporary Rails application in subprocesses and inspect loopback HTTP delivery, process status and stderr. They cover terminal metadata and stack data, successful and failing exits, executor opt-out, later exit-hook failures, disabled capture/client, exclusions, privacy-hook drops and failures, uninitialized clients, non-runner processes, deduplication and reporting failures. A stable supplied backtrace also checks stderr byte-for-byte against disabled capture.
Authoring-mode test audit covered every new example and parameterized case. Replacing the two production files with their pre-feature versions produced five expected failures. Temporarily bypassing
before_sendcaused both privacy-hook examples to fail on received events. All temporary mutations were restored. The audit also repaired those examples to keep reinitialized clients pointed at the asserted loopback receiver with uncompressed JSON requests. The mutation then failed at the intended no-events assertion, not at receiver parsing.Exact committed HEAD autoreview completed with Pi using
claude-code/claude-opus-5-5. It reported no findings. All commits are signed and locally verified.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileA hand-authored changeset file is included. 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. Local Pi session ID:01a1b0e4-9039-71a4-a297-fed90e502ce9. There is no public session URL.The implementation uses the existing exit hook and capture pipeline rather than adding another transport or public configuration setting. This draft requires human review. No self-approval, merge or auto-merge is requested.