Repository navigation
fix(flags): fall back for incompatible local regex evaluation - #231
Conversation
posthog-elixir Compliance ReportDate: 2026-10-23T10:47:41.839878+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 609ms |
| Format Validation.Event Has Uuid | ✅ | 608ms |
| Format Validation.Event Has Lib Properties | ✅ | 607ms |
| Format Validation.Distinct Id Is String | ✅ | 608ms |
| Format Validation.Token Is Present | ✅ | 608ms |
| Format Validation.Custom Properties Preserved | ✅ | 610ms |
| Format Validation.Event Has Timestamp | ✅ | 610ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ❌ | 613ms |
| Retry Behavior.Retries On 503 | ✅ | 5613ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2611ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2610ms |
| Retry Behavior.Respects Retry After Header | ✅ | 5614ms |
| Retry Behavior.Implements Backoff | ✅ | 15624ms |
| Retry Behavior.Retries On 500 | ✅ | 5614ms |
| Retry Behavior.Retries On 502 | ✅ | 5614ms |
| Retry Behavior.Retries On 504 | ✅ | 5613ms |
| Retry Behavior.Max Retries Respected | ✅ | 15624ms |
| Deduplication.Generates Unique Uuids | ✅ | 614ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5614ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10617ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5616ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 612ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 610ms |
| Compression.Sends Gzip When Enabled | ✅ | 608ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 608ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 605ms |
| Batch Format.Multiple Events Batched Together | ✅ | 611ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2611ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2610ms |
| Error Handling.Retries On 408 | ✅ | 5613ms |
Failures
format_validation.non_utc_event_timestamp_is_converted_to_utc
Event 0 field 'timestamp' instant '2026-10-23T10:45:57.857211Z' != expected '2025-01-02T03:04:05Z'
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 7ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 4ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 5ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 4ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 4ms |
| Request Payload.Groups Round Trip | ✅ | 5ms |
| Request Payload.Groups Default To Empty Object | ❌ | 4ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 5ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 4ms |
| 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 | ✅ | 605ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 9ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ❌ | 4ms |
| Retry Behavior.Retries Flags On 502 | ❌ | 308ms |
| Retry Behavior.Retries Flags On 504 | ❌ | 308ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ❌ | 607ms |
Failures
request_payload.groups_default_to_empty_object
Field 'groups' not found in /flags request body at path 'groups'. Available keys: ['api_key', 'distinct_id', 'flag_keys_to_evaluate']
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['api_key', 'distinct_id', 'flag_keys_to_evaluate']
request_lifecycle.mock_response_value_is_returned_to_caller
Last action result missing field 'value'. Keys: ['error', 'success']
retry_behavior.retries_flags_on_502
Last action result missing field 'value'. Keys: ['error', 'success']
retry_behavior.retries_flags_on_504
Last action result missing field 'value'. Keys: ['error', 'success']
side_effect_events.get_feature_flag_captures_feature_flag_called_event
Expected 1 events with name '$feature_flag_called', got 0
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 43c0ac1: 1 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
💡 Motivation and Context
Local feature flag evaluation can return a definitive PCRE result that disagrees with the flags service, preventing remote fallback. For example,
^foo$matchesfoo\nin default PCRE but not in the service. PCRE also treats\vas vertical whitespace, so it matches LF, CR and FF, while the service treats it as vertical tab only.This adapts the approach discussed in posthog-js#5354 and the cross-SDK agreement to Elixir's engine. It does not assume that the JavaScript change has been merged or released.
Keep known shared ASCII patterns local and use the existing inconclusive path for uncertain syntax or semantics. This deliberately reduces local coverage for non-ASCII values and patterns, Unicode/POSIX/set classes, advanced groups and flags, word boundaries, and the
\vescape. Normal callers can fall back remotely. Local-only callers leave affected flags unresolved. The strict end-anchor option keeps the final-LF case local with the service result. Empty patterns, escaped punctuation, literal/hex vertical tab and class backspace remain local.The ASCII input check now scans the binary tail-recursively and stops at the first non-ASCII byte. It does not first allocate a list of every byte. This changes allocation and early-exit cost, not which values resolve locally.
Execution now uses the compiled pattern with
:re.run,:report_errorsand no captures. With the default local runtime budget,^(a+)+$|^a+\n$on 30 ASCIIas plus LF exhausts the match limit before its matching alternative.Regex.match?/2had hidden this as false, so the old localregex/not_regexresults were false/true while the actual service returns true/false. Execution errors now stay inconclusive before either operator or cohort negation. The strict-dollar compile option, binary input scan, engine and default match budget are unchanged.Only proven shared compilation errors become definitive false results for both
regexandnot_regex. Other compile or execution failures remain inconclusive. A private invalid-pattern result is normalized before direct negation. Inside cohorts it remains inconclusive so leaf, group or reference negation cannot turn an invalid search into a match. No public API, dependency or regex engine is added.💚 How did you test it?
Regression tests failed before the original fix and before the vertical-tab correction. After integrating current main locally:
test/posthog/integrations/plug_test.exs: 750 passed, 18 excluded.A bounded before/after scanner benchmark used five fresh-process samples each at 1 KiB, 64 KiB and 1 MiB, with ASCII and leading/trailing non-ASCII inputs. The old 1 MiB byte list contains 16 MiB of list cells on this 64-bit BEAM. Median immediate process memory growth was 43,776,352 bytes before and zero observed growth after, for all three 1 MiB cases. This is retained heap growth, not a total-allocation measurement. Leading-byte fallback used 18,572 reductions before and 12 after. Timings were descriptive only. Normal tests have no timing or memory thresholds.
The execution-error regression used the actual default runtime and real evaluator before source changes, with no artificial compile limit or mocked engine. Four focused tests failed before the change and all 190 focused tests pass now. Coverage includes both operators, direct and nested/reference/leaf cohort negation, local-only omission, remote fallback, and retained invalid/ordinary local results.
Public-boundary tests intercept the existing HTTP client, not the regex engine. They check remote fallback, unresolved local-only results, a missing-property control, and ordinary ASCII patterns that must remain local.
Local runtime: Elixir 1.20.4 / OTP 29 / PCRE2 10.47. Older supported runtimes still need CI. The unfiltered suite is not locally green: seven unrelated Plug/Bandit/Cowboy tests previously failed because port 8001 is occupied. Those tests were not removed or changed, and no process was killed to free the port. Live-service tests were not run. No all-green CI claim is made. The existing posted compliance report lists seven failures outside this input-scan feedback change; this update neither fixes nor disproves them.
📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Directed by @marandaneto using a Pi implementation agent, local Git/gh/Mix tools, the frozen service oracle, and the isolated Pi autoreview helper. No public session link is available. A human review is still required.
The chosen approach is conservative fallback rather than a new regex engine or a Unicode-mode switch. Review caught the PCRE vertical-whitespace difference, which now defers in both class and non-class forms. Invalid-pattern handling stays private and preserves cohort error propagation.
PR review follow-up: PCRE verbs and misplaced quantifiers
Addressed the remaining suggestion in
8b442012a3ad0bd1c2120c4bb6f6bc4c8191de2e. The portability scanner now rejects*,+, and?after the start of the pattern, an opening group, or an alternation. This prevents PCRE-only verbs and controls such as(*FAIL),(*ACCEPT),(*COMMIT), and(*LIMIT_MATCH=1)from producing definitive local results. Both operators remain inconclusive through direct and cohort negation; ordinary callers can fall back remotely, and local-only snapshots omit the unresolved flag. Valid quantifiers, including lazy and grouped repetition, remain local.Contract note: the local evaluator spec specifies definitive false for syntactically invalid patterns. This conservative implementation deliberately defers misplaced quantifiers (including previously definitive
*and?*) and PCRE-only controls instead of attempting to validate them against the service grammar. Remote evaluation remains eligible; local-only callers receive unresolved results rather than definitive false. Proven shared invalid patterns such as[retain their existing false result. No new public API is introduced.Latest validation, superseding the earlier counts above:
mix test test/posthog/feature_flags/regex_compatibility_test.exs test/posthog/feature_flags/local_evaluation_integration_test.exsfailed 32 tests, including both public fallback regressions. The same command now passes all 226 tests.test/posthog/integrations/plug_test.exs: 786 passed, 18 excluded.MIX_ENV=test mix compile --warnings-as-errors, public API snapshot, andgit diff --checkpassed.