fix: Return None for malformed feature flag payloads - #947
marandaneto wants to merge 4 commits into
Conversation
posthog-python Compliance ReportDate: 2026-09-22T08:06:37.755802+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
|
idk if this one should be fixed or if the other sdks should be fixed |
JamesPatrickGill
left a comment
There was a problem hiding this comment.
Just a couple of small blocking points below. Looks good otherwise.
To answer your question I think None is the correct direction in this regard. We already have the API rejecting malformed payloads and the raw string is just a bit ambiguous.
|
The PR appears safe to merge, with a non-blocking observability concern around repeated payload parse warnings. Reviews (1) · Last reviewed commit: "fix: merge main and restore JSON import ..." |
| except (ValueError, RecursionError): | ||
| logging.getLogger("posthog").warning( | ||
| "[FEATURE FLAGS] Unable to parse flag payload as JSON" | ||
| ) |
There was a problem hiding this comment.
Payload warnings can flood logs
Every empty, whitespace-only, or persistently malformed payload emits this warning on every evaluation. Because empty payloads are supported inputs and warnings are not deduplicated, routine evaluations or one bad flag can flood application logs. Return None silently for empty payloads, and rate-limit, deduplicate, or lower the log level for repeated malformed payloads.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/types.py
Line: 17-20
Comment:
**Payload warnings can flood logs**
Every empty, whitespace-only, or persistently malformed payload emits this warning on every evaluation. Because empty payloads are supported inputs and warnings are not deduplicated, routine evaluations or one bad flag can flood application logs. Return `None` silently for empty payloads, and rate-limit, deduplicate, or lower the log level for repeated malformed payloads.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
💡 Motivation and Context
Malformed JSON payloads currently return the raw string through
evaluate_flags(...).get_flag_payload(...), while the deprecatedget_feature_flag_payload(...)can raiseJSONDecodeError.Both APIs now use a shared parser that returns
Nonefor malformed or empty JSON strings, for local and server-provided payloads. Valid JSON and pre-decoded values are preserved.💚 How did you test it?
3781203againstorigin/mainreported no actionable findings.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with Pi using file editing, shell tests, GitHub CLI, and the isolated autoreview helper. The requested behavior is to treat malformed JSON payloads as absent rather than return invalid strings or raise decoding errors. The change shares parsing between the newer and deprecated APIs and includes local and remote regression coverage. Human review is required.