fix(features): Honor registered handlers in batch checks - #123181
Conversation
github-actions
Bot
added
the
Scope: Backend
Sentry Snapshot Testing
|
| project_results: dict[Project, bool | None] = {} | ||
| remaining_projects = set(projects) | ||
| # Use the same logic as `has_for_batch` to evaluate the feature for each project in the batch | ||
| self._run_registered_handlers_for_batch( |
There was a problem hiding this comment.
unfortunately this implementation is a bit awkward. the obvious implementation is just to call has for all the flags we need to check, but the main advantage of batch_has is that we have some shared context (like project and organization) so we can be more efficient. to preserve that i decided to duplicate a decent bit of the logic of has itself
7147bf0 to
cfd1ff4
Compare
kenzoengineer
left a comment
kenzoengineer
left a comment
There was a problem hiding this comment.
sure there's some duplicated code but the logic still makes sense to me
Run registered handlers before the entity handler, matching has(). Isolate registered handler failures while evaluating fallthroughs in one entity batch. Leave unhandled flags absent so serializer fallbacks continue to own configured defaults. Co-authored-by: Claude <noreply@anthropic.com>
cfd1ff4 to
9befbc7
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9befbc7. Configure here.
| for project, project_value in project_results.items(): | ||
| project_key = f"project:{project.id}" | ||
| feature_results = registered_results.setdefault(project_key, {}) | ||
| feature_results[feature_name] = project_value |
There was a problem hiding this comment.
Partial project results block fallback
Medium Severity
When a registered handler resolves a feature for only some projects, those values are stored even though remaining projects are still unresolved. If the entity handler then omits those projects, batch_has returns a partial map. Callers that treat a present feature as fully batch-evaluated skip fallback, so unresolved projects never receive an entity or default decision.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9befbc7. Configure here.
2 participants
Right now,
batch_hasonly will check flagpole, rather than checking both registered flag handler in getsentry and then flagpole, likehasdoes. This makes it problematic to actually use getsentry and flagpole together — e.g. the org serializer won't check getsentry, which means that flags withapi_expose=Truewill not go through getsentry handlers.Change this so that for each feature flag called in
batch_has, we check whether it has a registered handler first, then batch evaluate everything else.