Repository navigation
feat(rails): capture Action Cable exceptions - #335
Open
marandaneto wants to merge 4 commits into
Open
marandaneto wants to merge 4 commits into
marandaneto wants to merge 4 commits into
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-07T07:04:55.130558+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 7ms |
| Format Validation.Event Has Uuid | ✅ | 4ms |
| Format Validation.Event Has Lib Properties | ✅ | 4ms |
| Format Validation.Distinct Id Is String | ✅ | 6ms |
| Format Validation.Token Is Present | ✅ | 4ms |
| Format Validation.Custom Properties Preserved | ✅ | 4ms |
| Format Validation.Event Has Timestamp | ✅ | 4ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 4ms |
| Retry Behavior.Retries On 503 | ✅ | 5292ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 6ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 5ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8014ms |
| Retry Behavior.Implements Backoff | ❌ | 351ms |
| Retry Behavior.Retries On 500 | ✅ | 5111ms |
| Retry Behavior.Retries On 502 | ✅ | 5112ms |
| Retry Behavior.Retries On 504 | ✅ | 5140ms |
| Retry Behavior.Max Retries Respected | ❌ | 567ms |
| Deduplication.Generates Unique Uuids | ✅ | 14ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5158ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10365ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5116ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 15ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 9ms |
| Compression.Sends Gzip When Enabled | ✅ | 5ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 4ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 2ms |
| Batch Format.Multiple Events Batched Together | ❌ | 11ms |
| Error Handling.Does Not Retry On 403 | ❌ | 3ms |
| Error Handling.Does Not Retry On 413 | ❌ | 3ms |
| Error Handling.Retries On 408 | ✅ | 5146ms |
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 | ✅ | 6ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 5ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 4ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 5ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 4ms |
| Request Payload.Groups Round Trip | ✅ | 4ms |
| Request Payload.Groups Default To Empty Object | ✅ | 5ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 7ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 8ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 4ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 4ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 6ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 4ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 107ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 152ms |
| 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:05:35.427397+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 9ms |
| Format Validation.Event Has Uuid | ✅ | 106ms |
| Format Validation.Event Has Lib Properties | ✅ | 107ms |
| Format Validation.Distinct Id Is String | ✅ | 107ms |
| Format Validation.Token Is Present | ✅ | 107ms |
| Format Validation.Custom Properties Preserved | ✅ | 106ms |
| Format Validation.Event Has Timestamp | ✅ | 107ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 8ms |
| Retry Behavior.Retries On 503 | ✅ | 5311ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2108ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2109ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8116ms |
| Retry Behavior.Implements Backoff | ✅ | 15719ms |
| Retry Behavior.Retries On 500 | ✅ | 5213ms |
| Retry Behavior.Retries On 502 | ✅ | 5212ms |
| Retry Behavior.Retries On 504 | ✅ | 5214ms |
| Retry Behavior.Max Retries Respected | ✅ | 15618ms |
| Deduplication.Generates Unique Uuids | ✅ | 112ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5211ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10410ms |
| 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 | ✅ | 105ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 106ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 5ms |
| Batch Format.Multiple Events Batched Together | ✅ | 111ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2108ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2109ms |
| Error Handling.Retries On 408 | ✅ | 5213ms |
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 | ✅ | 106ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 107ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 106ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 107ms |
| Request Payload.Groups Round Trip | ✅ | 106ms |
| 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 | ✅ | 106ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 3ms |
| 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 | ✅ | 107ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 237ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 209ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 109ms |
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:48
marandaneto
requested review from
a team,
JamesPatrickGill,
ablaszkiewicz and
hpouillot
November 6, 2026 14:48
![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 72ba70a: 1 Must fix, 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.
![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 |
dustinbyrne
reviewed
Nov 6, 2026
dustinbyrne
left a comment
dustinbyrne
left a comment
Contributor
There was a problem hiding this comment.
Two reproduced cases need correction before approval: Cable-only sensitive-parameter filters are omitted, and exceptions consumed by a connection's rescue_from are emitted as unhandled. The existing Action Cable examples pass but do not cover these cases.
AI-assisted review.
Member
Author
|
Addressed the filtering concern in comment 6687743695 and duplicate inline reports in 43046fc plus 983872c, now pushed with verified signatures. Application and effective Cable filters apply to both payloads, including nested values and supported Proc rules. Shared inherited rules are deduplicated to avoid undoing non-idempotent Proc scrubbing. Initial RED on reviewed code: 37 examples, 8 failures, including both leaking payloads. The shared-Proc example separately failed on a5e35b6 (1 example, 1 failure) and passes with the follow-up. Final |
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
Action Cable runs connection callbacks and channel actions outside the normal HTTP exception middleware. This adds automatic capture around connection open/close, channel subscribe/unsubscribe, and dispatched channel actions. It uses the existing
auto_capture_exceptionsoption, which remains false by default, and respectsexcluded_exceptions. No new public configuration or methods are added. The existing changeset selects a minor release ofposthog-railsonly.Each operation gets isolated context from the connection environment and restores the caller's context afterward. Authenticated controller or connection users take precedence over sanitized PostHog header identity. Existing user lookup options apply.
capture_user_context = falsedisables user lookup, anduse_tracing_headers = falsedisables header correlation. A controller resolver is never passed a connection. Explicit capture IDs and properties keep their existing precedence.Channel parameters and action data combine application
filter_parameterswith the effective connection'sconfig.filter_parameters, including nested values and supported Proc rules, before serialization. Duplicate inherited rules run once, so shared non-idempotent Proc filters cannot undo their own scrubbing. Connections without filter configuration fall back to application filters. Raw environments, cookies, authorization headers, subscription identifiers, and URL query strings are not attached. Applications should review filtering andbefore_sendfor their own sensitive data.Channel failures dispatched through
Subscriptions#execute_commandwait until connection rescue handling finishes. Errors consumed by application handlers, including channel-local and connectionrescue_from, are not captured by these boundaries, even withreport_rescued_exceptions = true. That option does not enable handled Action Cable events. Unhandled channel errors retain channel/action context, including when the dispatcher logs without re-raising. Direct channel calls capture escaping errors without invoking connection handlers. Returns, escaping exceptions, handler invocation counts, and capture deduplication remain unchanged.Action Cable remains optional and loads lazily. Its added dependency is development/test-only. The wrappers do not create or enrich spans. Rails 5.2 has a fallback load hook, but local validation covered Rails 7.2.3.1 and 8.1.4, not a Rails 5.2 application.
The relevant SDK specs cover exception metadata, tracing headers, exception capture, logs context, and
before_send. None defines an Action Cable-specific API. New events use sourcerails.action_cable, levelerror, and amiddlewaremechanism withhandled: false. One contract difference remains: this integration passes source and level through the existing Ruby exception property bag rather than a typed internal metadata channel. This PR does not redesign the core exception API.💚 How did you test it?
Validated on Ruby 4.0.7 and Bundler 4.0.13 after a signed merge of base
81093eaf8199fee1330758bb9940a4d17b2acfdc:72ba70aa210dad9f0bdb258e5825824d58a28fbc: the final 37 Action Cable examples had 8 expected behavioral failures. Both payloads leaked Cable-only nested secrets, consumed subscribe/message/unsubscribe errors were captured, and escaping handler errors retained the wrong exception. Those 37 examples pass with the fix.a5e35b6with 1 example, 1 failure: a reversing Proc ran twice and restored the original value.bundle exec rspec spec/posthog/rails/action_cable_spec.rb --example 'runs a shared application Proc'now passes with 1 example, 0 failures. The full focused Action Cable suite passes with 38 examples, 0 failures.execute_commandcoverage includes handler invocation counts, direct calls, channel-local rescue, authenticated identity, successful returns, concurrent dispatch isolation, client gates, filtering failures, deduplication, andbefore_senddrops. Subprocess cases cover application boot and lazy loading.bundle exec rubocop: 152 files, no offenses.bundle exec rake public_api:checkandgit diff --check: passed. No public API snapshot change was needed.The diff-scoped cleanup and Authoring test audit found no blockers. Tests use independently expected event payloads and actual framework lifecycle paths. The historical Ruby/Rails matrix was not run locally.
Exact-HEAD autoreview on
983872c1b0d5239e7e332f133e0a4a590570dbe0againstorigin/maincompleted with no findings.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe existing hand-authored
.changeset/rails-action-cable-exceptions.mdis included.pnpm changesetwas not run, and no duplicate changeset or feature version bump was added.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Human DRI: @marandaneto. Pi's worker prepared the feature and addressed approved review feedback using local file tools, Bash, Git, GitHub CLI, RSpec, RuboCop, Rake, and the
autoreviewhelper withclaude-code/claude-opus-5-5. There is no public session URL.The implementation keeps automatic capture opt-in and preserves the existing policy of not autocapturing application-consumed Cable errors. It waits for actual connection handler execution rather than checking whether a handler exists. This PR requires human review before merge.