Repository navigation
fix(flags): treat failed remote records as unevaluated - #359
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-13T09:17:42.069464+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 10ms |
| Format Validation.Event Has Uuid | ✅ | 7ms |
| Format Validation.Event Has Lib Properties | ✅ | 6ms |
| Format Validation.Distinct Id Is String | ✅ | 9ms |
| Format Validation.Token Is Present | ✅ | 8ms |
| Format Validation.Custom Properties Preserved | ✅ | 8ms |
| Format Validation.Event Has Timestamp | ✅ | 7ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 7ms |
| Retry Behavior.Retries On 503 | ✅ | 5308ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 7ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 6ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8012ms |
| Retry Behavior.Implements Backoff | ❌ | 356ms |
| Retry Behavior.Retries On 500 | ✅ | 5156ms |
| Retry Behavior.Retries On 502 | ✅ | 5142ms |
| Retry Behavior.Retries On 504 | ✅ | 5116ms |
| Retry Behavior.Max Retries Respected | ❌ | 566ms |
| Deduplication.Generates Unique Uuids | ✅ | 18ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5153ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10270ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5163ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 18ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 10ms |
| Compression.Sends Gzip When Enabled | ✅ | 6ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 6ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 4ms |
| Batch Format.Multiple Events Batched Together | ❌ | 17ms |
| Error Handling.Does Not Retry On 403 | ❌ | 4ms |
| Error Handling.Does Not Retry On 413 | ❌ | 5ms |
| Error Handling.Retries On 408 | ✅ | 5145ms |
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 | ✅ | 8ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 7ms |
| Request Payload.Groups Round Trip | ✅ | 7ms |
| Request Payload.Groups Default To Empty Object | ✅ | 7ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 7ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 6ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 7ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 6ms |
| 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 | ✅ | 108ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 109ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 9ms |
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-13T09:18:31.752957+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 108ms |
| 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 | ✅ | 107ms |
| Format Validation.Custom Properties Preserved | ✅ | 109ms |
| Format Validation.Event Has Timestamp | ✅ | 108ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 108ms |
| Retry Behavior.Retries On 503 | ✅ | 5413ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2110ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2110ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8116ms |
| Retry Behavior.Implements Backoff | ✅ | 15612ms |
| Retry Behavior.Retries On 500 | ✅ | 5212ms |
| Retry Behavior.Retries On 502 | ✅ | 5214ms |
| Retry Behavior.Retries On 504 | ✅ | 5213ms |
| Retry Behavior.Max Retries Respected | ✅ | 15623ms |
| Deduplication.Generates Unique Uuids | ✅ | 111ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5211ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10316ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5216ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 112ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 108ms |
| Compression.Sends Gzip When Enabled | ✅ | 106ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 107ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 5ms |
| Batch Format.Multiple Events Batched Together | ✅ | 112ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2112ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2110ms |
| Error Handling.Retries On 408 | ✅ | 5211ms |
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 | ✅ | 108ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 108ms |
| Request Payload.Groups Round Trip | ✅ | 108ms |
| Request Payload.Groups Default To Empty Object | ✅ | 108ms |
| 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 | ✅ | 108ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 3ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 107ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 113ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 109ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 210ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 211ms |
| 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 |
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.
ioannisj
left a comment
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
Nothing blocking from me. The direct reads, bulk reads and the local-then-remote fallback line up with the sdk-specs #211 scenarios, and I couldn't find a path where the new nil values crash or mislabel a healthy flag. One small suggestion inline about the new error string.
Motivation and context
Implements sdk-specs #211: detailed failed records are unevaluated, not healthy disabled flags.
niland reportevaluation_failedwith a null response; response-wide errors remain independent.FeatureFlagError::EVALUATION_FAILEDand both producers.No public API/dependency changes or new typed-v4 support. Existing SDK patch intent and release-tool-managed Rails dependency patch retained; no manual bump or release.
Scope limit: original snapshots still report
evaluation_failedfor a locally unresolved key with failed remote diagnostics, rather than the evaluate-flags inconclusive diagnostic. That existing telemetry divergence remains a separate contract follow-up, not a conformance claim.How did you test it?
At signed commit
3627a97231ded3b9a04fde53bf6a18c3cefe161d, Ruby 4.0.7 / locked dependencies:autoreview --mode branch --base origin/main: clean; TruffleHog clean.No full Ruby/platform matrix, optional live OpenTelemetry/Rails integrations, gem build, benchmark or shared-harness claim. CI was not awaited.
Checklist
Agent context
Autonomy: Human-driven (agent-assisted). Human DRI and assignee: @marandaneto. Pi implemented the approved changes and ran the checks and isolated review above. Human review is required before merge; no auto-merge or release requested.