Repository navigation
fix(flags): treat failed remote records as unevaluated - #359
marandaneto wants to merge 2 commits into
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-12T13:43:03.769039+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 9ms |
| Format Validation.Event Has Uuid | ✅ | 7ms |
| Format Validation.Event Has Lib Properties | ✅ | 6ms |
| Format Validation.Distinct Id Is String | ✅ | 10ms |
| Format Validation.Token Is Present | ✅ | 10ms |
| 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 | ✅ | 5330ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 7ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 6ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8015ms |
| Retry Behavior.Implements Backoff | ❌ | 245ms |
| Retry Behavior.Retries On 500 | ✅ | 5115ms |
| Retry Behavior.Retries On 502 | ✅ | 5152ms |
| Retry Behavior.Retries On 504 | ✅ | 5150ms |
| Retry Behavior.Max Retries Respected | ❌ | 675ms |
| Deduplication.Generates Unique Uuids | ✅ | 16ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5142ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10378ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5168ms |
| 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 | ✅ | 2ms |
| Batch Format.Multiple Events Batched Together | ❌ | 15ms |
| Error Handling.Does Not Retry On 403 | ❌ | 5ms |
| Error Handling.Does Not Retry On 413 | ❌ | 4ms |
| Error Handling.Retries On 408 | ✅ | 5162ms |
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 | ✅ | 8ms |
| 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 | ✅ | 6ms |
| Request Payload.Groups Round Trip | ✅ | 6ms |
| Request Payload.Groups Default To Empty Object | ✅ | 6ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 5ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 6ms |
| 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 | ✅ | 6ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 108ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 110ms |
| 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-12T13:43:41.541464+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 111ms |
| Format Validation.Event Has Uuid | ✅ | 107ms |
| Format Validation.Event Has Lib Properties | ✅ | 109ms |
| Format Validation.Distinct Id Is String | ✅ | 107ms |
| Format Validation.Token Is Present | ✅ | 108ms |
| Format Validation.Custom Properties Preserved | ✅ | 109ms |
| Format Validation.Event Has Timestamp | ✅ | 107ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 9ms |
| Retry Behavior.Retries On 503 | ✅ | 5311ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2109ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2109ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8116ms |
| Retry Behavior.Implements Backoff | ✅ | 15622ms |
| Retry Behavior.Retries On 500 | ✅ | 5213ms |
| Retry Behavior.Retries On 502 | ✅ | 5214ms |
| Retry Behavior.Retries On 504 | ✅ | 5212ms |
| Retry Behavior.Max Retries Respected | ✅ | 15623ms |
| Deduplication.Generates Unique Uuids | ✅ | 112ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5211ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10419ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5213ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 111ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 108ms |
| Compression.Sends Gzip When Enabled | ✅ | 108ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 107ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 6ms |
| 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 | ✅ | 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 | ✅ | 108ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 107ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 108ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 107ms |
| Request Payload.Groups Round Trip | ✅ | 108ms |
| Request Payload.Groups Default To Empty Object | ✅ | 107ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 108ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 107ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 109ms |
| 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 | ✅ | 114ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 107ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 210ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 210ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 110ms |
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 |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed c00a32d: 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.
💡 Motivation and Context
A detailed
/flagsrecord markedfailed: truemeans the server produced no evaluated value, not a disabled flag. This implements the direct failed-record contract in PostHog/sdk-specs#211 while preserving successful same-context local decisions:niland reportevaluation_failedwith a null response, independently of the response-wide error bit.No public API or dependency changes. The existing patch changeset targets
posthog-ruby; release tooling handles the dependency-only Rails patch. No manual bumps or duplicate release intent. This uses the existing detailed v2 path and adds no typed-v4 support.Explicit scope limit: the existing snapshot
evaluation_failedversuslocal_evaluation_inconclusiveevent policy is unchanged and remains a separate contract follow-up. This PR does not claim full snapshot-telemetry conformance or introduce another snapshot-inclusion policy.💚 How did you test it?
Current local result at
9e642519cf69529338a8c6a39cf5b399e8a93d85, Ruby 4.0.7 / Bundler 4.0.13 with locked dependencies:checkout => experience_continuity, got{}. The identical focused command passes after the one-line filter fix.bundle exec rspec: 2,526 examples, 0 failures, 49 pending optional OpenTelemetry integrations.bundle exec rubocop: 167 files, no offenses.bundle exec rake public_api:checkandgit diff --check: passed, no API snapshot changes.origin/main(669aabfae651c88a3bf3a354ff0e876bc7576044): clean, no accepted/actionable findings. Diff-scoped Authoring audit and deslop completed.The initial default gem environment lacked locked rack/crass; validation reused the existing external gem cache with no installation or lockfile change. Supported Ruby/platform CI matrix, optional real OpenTelemetry/latest-Rails bundle, package builds, benchmarks and shared cross-SDK harness were not run this round. WebMock delivery is not live-service coverage. Earlier general compliance/performance reports are not new verified regressions or a current green-CI claim.
📝 Checklist
🤖 Agent context
Autonomy: Human-driven (agent-assisted). Human DRI: @marandaneto. Pi implemented the approved fixes and ran the tests, source/test audit, and isolated autoreview listed above. Human review is required before merge. No shareable session link is available.