Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 43
fix: isolate provider event handler dispatch#599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
gruebel
merged 4 commits into
open-feature:main
from
Lucas-FManager:code/openfeature-event-handler-isolation-596May 29, 2026
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b52974c
Isolate provider event handlers
Lucas-FManager dcf837a
Address event handler review feedback
Lucas-FManager 8b1c1f3
test: cover event dispatch noop path
Lucas-FManager a049b00
fixup: drain executor at exit and relax non-blocking test timing margin
toddbaert File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import inspect | ||
| import threading | ||
| import time | ||
| import types | ||
| import uuid | ||
| @@ -7,7 +8,7 @@ | ||
| import pytest | ||
| from openfeature import api | ||
| from openfeature import _event_support, api | ||
| from openfeature.api import ( | ||
| add_hooks, | ||
| clear_hooks, | ||
| @@ -29,6 +30,15 @@ | ||
| from openfeature.transaction_context import ContextVarsTransactionContextPropagator | ||
| def wait_for_mock_call(mock: MagicMock, timeout: float = 1.0) -> None: | ||
| deadline = time.monotonic() + timeout | ||
| while time.monotonic() < deadline: | ||
| if mock.call_count: | ||
| return | ||
| time.sleep(0.01) | ||
| @pytest.mark.parametrize( | ||
| "flag_type, default_value, get_method", | ||
| ( | ||
| @@ -467,6 +477,10 @@ def emit_all_events(provider): | ||
| # Then | ||
| # NOTE: provider_ready is called immediately after adding the handler | ||
| wait_for_mock_call(spy.provider_ready) | ||
| wait_for_mock_call(spy.provider_configuration_changed) | ||
| wait_for_mock_call(spy.provider_error) | ||
| wait_for_mock_call(spy.provider_stale) | ||
| spy.provider_ready.assert_called_once() | ||
| spy.provider_configuration_changed.assert_called_once_with(details) | ||
| spy.provider_error.assert_called_once_with(details) | ||
| @@ -525,9 +539,25 @@ def test_provider_event_late_binding(): | ||
| other_provider.emit_provider_configuration_changed(other_provider_details) | ||
| # Then | ||
| wait_for_mock_call(spy.provider_configuration_changed) | ||
| spy.provider_configuration_changed.assert_called_once_with(details) | ||
| def test_run_client_handlers_without_registered_handlers_is_noop(): | ||
| provider = NoOpProvider() | ||
| set_provider(provider) | ||
| client = get_client("client-without-handlers") | ||
| details = EventDetails(provider_name=provider.get_metadata().name) | ||
| assert client not in _event_support._client_handlers | ||
| _event_support.run_client_handlers( | ||
| client, ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details | ||
| ) | ||
| assert client not in _event_support._client_handlers | ||
| # Requirement 5.1.4, Requirement 5.1.5 | ||
| def test_provider_event_handler_exception(): | ||
| # Given | ||
| @@ -545,6 +575,7 @@ def test_provider_event_handler_exception(): | ||
| ) | ||
| # Then | ||
| wait_for_mock_call(spy.provider_error) | ||
| spy.provider_error.assert_called_once_with( | ||
| EventDetails( | ||
| flags_changed=None, | ||
| @@ -556,6 +587,68 @@ def test_provider_event_handler_exception(): | ||
| ) | ||
| def test_provider_event_handler_exception_does_not_stop_subsequent_handlers(): | ||
| # Given | ||
| provider = NoOpProvider() | ||
| set_provider(provider) | ||
| spy = MagicMock() | ||
| handler_called = threading.Event() | ||
| raising_handler = MagicMock(side_effect=RuntimeError("handler failed")) | ||
| def recording_handler(details): | ||
| spy.provider_error(details) | ||
| handler_called.set() | ||
| client = get_client() | ||
| client.add_handler(ProviderEvent.PROVIDER_ERROR, raising_handler) | ||
| client.add_handler(ProviderEvent.PROVIDER_ERROR, recording_handler) | ||
| details = ProviderEventDetails(error_code=ErrorCode.GENERAL, message="some_error") | ||
| expected_details = EventDetails.from_provider_event_details( | ||
| provider.get_metadata().name, details | ||
| ) | ||
| # When | ||
| provider.emit_provider_error(details) | ||
| # Then | ||
| assert handler_called.wait(timeout=1) | ||
gruebel marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| raising_handler.assert_called_once_with(expected_details) | ||
| spy.provider_error.assert_called_once_with(expected_details) | ||
| def test_provider_event_handlers_do_not_block_emitter(): | ||
| # Given | ||
| provider = NoOpProvider() | ||
| set_provider(provider) | ||
| handler_started = threading.Event() | ||
| release_handler = threading.Event() | ||
| handler_finished = threading.Event() | ||
| def slow_handler(details): | ||
| handler_started.set() | ||
| release_handler.wait(timeout=1) | ||
| handler_finished.set() | ||
| client = get_client() | ||
| client.add_handler(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, slow_handler) | ||
| # When | ||
| start_time = time.perf_counter() | ||
| provider.emit_provider_configuration_changed(ProviderEventDetails()) | ||
| elapsed = time.perf_counter() - start_time | ||
| # Then | ||
| assert handler_started.wait(timeout=1) | ||
| # emit must return well before the handler's blocking wait (1s) would finish | ||
| assert elapsed < 0.5 | ||
| release_handler.set() | ||
| assert handler_finished.wait(timeout=1) | ||
| def test_client_handlers_thread_safety(): | ||
| provider = NoOpProvider() | ||
| set_provider(provider) | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.