fix: isolate provider event handler dispatch by Lucas-FManager · Pull Request #599 · open-feature/python-sdk

Lucas-FManager · 2026-05-27T18:34:46Z

Problem

Provider event handlers currently execute synchronously while the handler registry lock is held. That means a handler exception can stop later handlers, and a slow handler can block the provider/client path that emitted the event.

Approach

Snapshot global/client handlers under the lock, then release the lock before dispatch.
Dispatch handlers through a shared ThreadPoolExecutor.
Wrap each handler invocation so one failing handler is logged and does not prevent the next handler from running.
Avoid the nested client-handler lock pattern in provider dispatch by snapshotting matching clients first.
Update event tests to wait for async handler dispatch and add coverage for error isolation and non-blocking emission.

Security

This avoids running arbitrary user event-handler code while internal handler registries are locked. It also keeps handler exceptions contained to the callback boundary instead of propagating into SDK/provider control flow.

Verification

uv run pytest tests -q -> 158 passed
uvx ruff check openfeature/_event_support.py tests/test_client.py tests/test_api.py -> passed
uvx ruff format --check openfeature/_event_support.py tests/test_client.py tests/test_api.py -> passed
git diff --check -> passed

Fixes #596

gemini-code-assist

Code Review

This pull request introduces asynchronous event handling using a ThreadPoolExecutor to execute OpenFeature event handlers, ensuring they do not block the emitter or other handlers. It also refactors locking mechanisms to release locks before handler execution and wraps handler execution in try-except blocks to prevent a single failing handler from halting subsequent ones. The review feedback recommends avoiding unnecessary mutation and potential memory leaks in defaultdict lookups by checking if keys exist before accessing them in both client and global handler registries.

Lucas-FManager · 2026-05-27T18:38:41Z

Updated in the latest push: run_client_handlers() now uses _client_handlers.get(client) and then handlers_by_event.get(event, ()), and run_global_handlers() uses _global_handlers.get(event, ()). That avoids mutating the defaultdict during read-only dispatch while preserving the async/error-isolated behavior.

Re-verified after the change:

uv run pytest tests -q -> 158 passed
uvx ruff check openfeature/_event_support.py tests/test_client.py tests/test_api.py -> passed
uvx ruff format --check openfeature/_event_support.py tests/test_client.py tests/test_api.py -> passed
git diff --check -> passed

gruebel

overall looks good and addresses the mentioned issues. added just a few comments, thanks 🍻

codecov · 2026-05-29T05:37:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.34%. Comparing base (cbacef0) to head (a049b00).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #599      +/-   ##
==========================================
+ Coverage   98.28%   98.34%   +0.06%     
==========================================
  Files          45       45              
  Lines        2386     2477      +91     
==========================================
+ Hits         2345     2436      +91     
  Misses         41       41

Flag	Coverage Δ
unittests	`98.34% <100.00%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Lucas-FManager · 2026-05-29T07:52:27Z

Small follow-up after the Codecov report: I added a focused regression test for the read-only client-dispatch no-op path, so _event_support.py is locally covered at 100%.

Verification after the new commit:

uv run pytest tests/test_client.py tests/test_api.py -q -> 74 passed
uv run coverage run -m pytest tests/test_client.py tests/test_api.py -q; uv run coverage report -m openfeature/_event_support.py -> _event_support.py 100%
uvx prek run --all-files --show-diff-on-failure -> passed

The main build workflow is still waiting for maintainer approval for the fork run, so there is no new CI failure on the latest head yet.

toddbaert · 2026-05-29T19:02:23Z

I made some small test improvements, and also shut down the executor on exit.

Signed-off-by: Lucas-FManager <[email protected]>

Signed-off-by: Todd Baert <[email protected]>

* Isolate provider event handlers Signed-off-by: Lucas-FManager <[email protected]> * Address event handler review feedback Signed-off-by: Lucas-FManager <[email protected]> * test: cover event dispatch noop path Signed-off-by: Lucas-FManager <[email protected]> * fixup: drain executor at exit and relax non-blocking test timing margin Signed-off-by: Todd Baert <[email protected]> --------- Signed-off-by: Lucas-FManager <[email protected]> Signed-off-by: Todd Baert <[email protected]> Co-authored-by: Lucas-FManager <[email protected]> Co-authored-by: Todd Baert <[email protected]> Signed-off-by: gruebel <[email protected]>

* test(e2e): add Behave step definitions for context merging Add the missing E2E step definitions so the contextMerging.feature scenarios from the OpenFeature spec run against python-sdk. Fixes #500 Changes: - Bump spec submodule to 130df3eb so contextMerging.feature is copied in during the `poe e2e` task. - Add tests/features/environment.py with a before_scenario hook that resets provider/hook/API-context/transaction-context state, so scenarios cannot leak state between features. - Add tests/features/steps/context_merging_steps.py: - RetrievableContextProvider captures the merged EvaluationContext it receives, so assertions can inspect what the SDK merged. - Step definitions for all scenarios in contextMerging.feature: single-level insert, multi-level insert, and per-key overwrite precedence across API / Transaction / Client / Invocation / Before Hooks. - Client-level context is set via direct attribute assignment on OpenFeatureClient.context (no new setter), since merging already honors client.context (openfeature/client.py:422-429). Runs clean: 4 features / 50 scenarios / 233 steps. Signed-off-by: gruebel <[email protected]> * docs: fix inaccuracies in README code examples (#592) Signed-off-by: Jonathan Norris <[email protected]> Signed-off-by: gruebel <[email protected]> * fix: correctly reset api state on shutdown (#589) correctly reset api state on shutdown Signed-off-by: gruebel <[email protected]> * chore(main): release 0.9.0 (#555) Signed-off-by: OpenFeature Bot <[email protected]> Signed-off-by: gruebel <[email protected]> * chore(deps): update pre-commit hook tox-dev/pyproject-fmt to v2.21.2 (#601) * chore(deps): update pre-commit hook tox-dev/pyproject-fmt to v2.21.2 * upper bound toml-fmt-common till fixed Signed-off-by: gruebel <[email protected]> --------- Signed-off-by: gruebel <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <[email protected]> Signed-off-by: gruebel <[email protected]> * chore(deps): update astral-sh/setup-uv action to v8 (#603) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <[email protected]> * chore(deps): update codecov/codecov-action action to v6 (#604) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <[email protected]> * chore(deps): update googleapis/release-please-action action to v5 (#605) * chore(deps): update googleapis/release-please-action action to v5 * fix config Signed-off-by: gruebel <[email protected]> --------- Signed-off-by: gruebel <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <[email protected]> Signed-off-by: gruebel <[email protected]> * chore(deps): update pre-commit hook pre-commit/mirrors-mypy to v2 (#606) * chore(deps): update pre-commit hook pre-commit/mirrors-mypy to v2 * update config Signed-off-by: gruebel <[email protected]> --------- Signed-off-by: gruebel <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <[email protected]> Signed-off-by: gruebel <[email protected]> * chore(deps): update dependency prek to >=0.4.3,<0.5.0 (#607) * chore(deps): update dependency prek to >=0.4.3,<0.5.0 * adjust CI Signed-off-by: gruebel <[email protected]> --------- Signed-off-by: gruebel <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <[email protected]> Signed-off-by: gruebel <[email protected]> * feat!: make set_provider non-blocking, add set_provider_and_wait (#595) * feat!: make set_provider non-blocking, add set_provider_and_wait Signed-off-by: Jonathan Norris <[email protected]> * fix: ruff format signature collapse in api.py Signed-off-by: Jonathan Norris <[email protected]> * fix: use threading.Event in error event test to avoid flaky busy-wait Signed-off-by: Jonathan Norris <[email protected]> * fixup: pr feedback and additional checks Signed-off-by: Todd Baert <[email protected]> * fix: check active registration in stale-init guard, not _provider_status Signed-off-by: Jonathan Norris <[email protected]> * fixup: edge shutdown race Signed-off-by: Todd Baert <[email protected]> --------- Signed-off-by: Jonathan Norris <[email protected]> Signed-off-by: Todd Baert <[email protected]> Co-authored-by: Todd Baert <[email protected]> Signed-off-by: gruebel <[email protected]> * fix: isolate provider event handler dispatch (#599) * Isolate provider event handlers Signed-off-by: Lucas-FManager <[email protected]> * Address event handler review feedback Signed-off-by: Lucas-FManager <[email protected]> * test: cover event dispatch noop path Signed-off-by: Lucas-FManager <[email protected]> * fixup: drain executor at exit and relax non-blocking test timing margin Signed-off-by: Todd Baert <[email protected]> --------- Signed-off-by: Lucas-FManager <[email protected]> Signed-off-by: Todd Baert <[email protected]> Co-authored-by: Lucas-FManager <[email protected]> Co-authored-by: Todd Baert <[email protected]> Signed-off-by: gruebel <[email protected]> * test: fix flaky event handler test (#609) fix flaky event handler test Signed-off-by: gruebel <[email protected]> * chore(deps): update pre-commit hook astral-sh/ruff-pre-commit to v0.15.15 (#608) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <[email protected]> * chore(main): release 0.10.0 (#602) * chore(main): release 0.10.0 Signed-off-by: OpenFeature Bot <[email protected]> * docs: clarify non-blocking set_provider behavior in changelog Signed-off-by: Todd Baert <[email protected]> --------- Signed-off-by: OpenFeature Bot <[email protected]> Signed-off-by: Todd Baert <[email protected]> Co-authored-by: Todd Baert <[email protected]> Signed-off-by: gruebel <[email protected]> * fix CR comments Signed-off-by: gruebel <[email protected]> * cleanup Signed-off-by: gruebel <[email protected]> --------- Signed-off-by: gruebel <[email protected]> Signed-off-by: Jonathan Norris <[email protected]> Signed-off-by: OpenFeature Bot <[email protected]> Signed-off-by: Todd Baert <[email protected]> Signed-off-by: Lucas-FManager <[email protected]> Co-authored-by: Jonathan Norris <[email protected]> Co-authored-by: Anton Grübel <[email protected]> Co-authored-by: OpenFeature Bot <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Todd Baert <[email protected]> Co-authored-by: Nguyen Cat Luong <[email protected]> Co-authored-by: Lucas-FManager <[email protected]>

Lucas-FManager requested review from a team as code owners May 27, 2026 18:34

gemini-code-assist Bot reviewed May 27, 2026

View reviewed changes

Comment thread openfeature/_event_support.py Outdated

Comment thread openfeature/_event_support.py Outdated

Lucas-FManager force-pushed the code/openfeature-event-handler-isolation-596 branch from 13de63c to 56e5045 Compare May 27, 2026 18:37

Lucas-FManager changed the title ~~Isolate provider event handler dispatch~~ fix: isolate provider event handler dispatch May 27, 2026

Lucas-FManager force-pushed the code/openfeature-event-handler-isolation-596 branch from 56e5045 to fb4e9a1 Compare May 27, 2026 18:38

This was referenced May 28, 2026

fix: dispatch event handlers asynchronously #598

Closed

fix: isolate event handler errors per spec 5.2.5 #600

Closed

gruebel reviewed May 28, 2026

View reviewed changes

Comment thread openfeature/_event_support.py Outdated

Comment thread openfeature/_event_support.py

Comment thread tests/test_client.py

Lucas-FManager force-pushed the code/openfeature-event-handler-isolation-596 branch from 0db2460 to 5dfdfd9 Compare May 28, 2026 23:47

toddbaert reviewed May 29, 2026

View reviewed changes

Comment thread .pre-commit-config.yaml Outdated

toddbaert requested a review from gruebel May 29, 2026 19:02

toddbaert force-pushed the code/openfeature-event-handler-isolation-596 branch from 0f441d7 to 2bd3e58 Compare May 29, 2026 19:20

Lucas-FManager and others added 4 commits May 29, 2026 15:23

Isolate provider event handlers

b52974c

Signed-off-by: Lucas-FManager <[email protected]>

Address event handler review feedback

dcf837a

Signed-off-by: Lucas-FManager <[email protected]>

test: cover event dispatch noop path

8b1c1f3

Signed-off-by: Lucas-FManager <[email protected]>

fixup: drain executor at exit and relax non-blocking test timing margin

a049b00

Signed-off-by: Todd Baert <[email protected]>

toddbaert force-pushed the code/openfeature-event-handler-isolation-596 branch from 2bd3e58 to a049b00 Compare May 29, 2026 19:23

toddbaert self-requested a review May 29, 2026 19:24

toddbaert approved these changes May 29, 2026

View reviewed changes

toddbaert requested review from federicobond and jonathannorris May 29, 2026 19:37

gruebel approved these changes May 29, 2026

View reviewed changes

gruebel merged commit 0a96426 into open-feature:main May 29, 2026
15 checks passed

openfeaturebot mentioned this pull request May 29, 2026

chore(main): release 0.10.0 #602

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: isolate provider event handler dispatch#599

fix: isolate provider event handler dispatch#599
gruebel merged 4 commits into
open-feature:mainfrom
Lucas-FManager:code/openfeature-event-handler-isolation-596

Lucas-FManager commented May 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Lucas-FManager commented May 27, 2026

Uh oh!

gruebel left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 29, 2026 •

edited

Loading

Uh oh!

Lucas-FManager commented May 29, 2026

Uh oh!

Uh oh!

toddbaert commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Lucas-FManager commented May 27, 2026

Problem

Approach

Security

Verification

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Lucas-FManager commented May 27, 2026

Uh oh!

gruebel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Lucas-FManager commented May 29, 2026

Uh oh!

Uh oh!

toddbaert commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented May 29, 2026 •

edited

Loading