From 34f9acde30a701a54b1d9605fb5ef307b6c26050 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 29 Jan 2025 16:28:52 +0100 Subject: [PATCH 1/3] Finally hooks do not get called when the provider is not ready #424 Signed-off-by: christian.lutnik --- openfeature/client.py | 62 ++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/openfeature/client.py b/openfeature/client.py index 1edfca63..fc6e65a0 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -295,37 +295,39 @@ def evaluate_flag_details( # noqa: PLR0915 reversed_merged_hooks = merged_hooks[:] reversed_merged_hooks.reverse() - status = self.get_provider_status() - if status == ProviderStatus.NOT_READY: - error_hooks( - flag_type, - hook_context, - ProviderNotReadyError(), - reversed_merged_hooks, - hook_hints, - ) - return FlagEvaluationDetails( - flag_key=flag_key, - value=default_value, - reason=Reason.ERROR, - error_code=ErrorCode.PROVIDER_NOT_READY, - ) - if status == ProviderStatus.FATAL: - error_hooks( - flag_type, - hook_context, - ProviderFatalError(), - reversed_merged_hooks, - hook_hints, - ) - return FlagEvaluationDetails( - flag_key=flag_key, - value=default_value, - reason=Reason.ERROR, - error_code=ErrorCode.PROVIDER_FATAL, - ) - try: + status = self.get_provider_status() + if status == ProviderStatus.NOT_READY: + error_hooks( + flag_type, + hook_context, + ProviderNotReadyError(), + reversed_merged_hooks, + hook_hints, + ) + flag_evaluation = FlagEvaluationDetails( + flag_key=flag_key, + value=default_value, + reason=Reason.ERROR, + error_code=ErrorCode.PROVIDER_NOT_READY, + ) + return flag_evaluation + if status == ProviderStatus.FATAL: + error_hooks( + flag_type, + hook_context, + ProviderFatalError(), + reversed_merged_hooks, + hook_hints, + ) + flag_evaluation = FlagEvaluationDetails( + flag_key=flag_key, + value=default_value, + reason=Reason.ERROR, + error_code=ErrorCode.PROVIDER_FATAL, + ) + return flag_evaluation + # https://github.com/open-feature/spec/blob/main/specification/sections/03-evaluation-context.md # Any resulting evaluation context from a before hook will overwrite # duplicate fields defined globally, on the client, or in the invocation. From c5ea1e6ab1914234794c48ee597375f32f6cd95b Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 29 Jan 2025 16:46:48 +0100 Subject: [PATCH 2/3] fixup! Finally hooks do not get called when the provider is not ready #424 Signed-off-by: christian.lutnik --- tests/test_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 7f0ca461..c829e9bc 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -221,7 +221,7 @@ def test_should_shortcircuit_if_provider_is_not_ready( assert flag_details.reason == Reason.ERROR assert flag_details.error_code == ErrorCode.PROVIDER_NOT_READY spy_hook.error.assert_called_once() - + spy_hook.finally_after.assert_called_once() # Requirement 1.7.7 def test_should_shortcircuit_if_provider_is_in_irrecoverable_error_state( @@ -243,7 +243,7 @@ def test_should_shortcircuit_if_provider_is_in_irrecoverable_error_state( assert flag_details.reason == Reason.ERROR assert flag_details.error_code == ErrorCode.PROVIDER_FATAL spy_hook.error.assert_called_once() - + spy_hook.finally_after.assert_called_once() def test_should_run_error_hooks_if_provider_returns_resolution_with_error_code(): # Given From 5524652bbbb4056e42d2c87666b0bed73aa88f5d Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 29 Jan 2025 16:50:51 +0100 Subject: [PATCH 3/3] fixup! Finally hooks do not get called when the provider is not ready #424 Signed-off-by: christian.lutnik --- tests/test_client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_client.py b/tests/test_client.py index c829e9bc..f6002c18 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -223,6 +223,7 @@ def test_should_shortcircuit_if_provider_is_not_ready( spy_hook.error.assert_called_once() spy_hook.finally_after.assert_called_once() + # Requirement 1.7.7 def test_should_shortcircuit_if_provider_is_in_irrecoverable_error_state( no_op_provider_client, monkeypatch @@ -245,6 +246,7 @@ def test_should_shortcircuit_if_provider_is_in_irrecoverable_error_state( spy_hook.error.assert_called_once() spy_hook.finally_after.assert_called_once() + def test_should_run_error_hooks_if_provider_returns_resolution_with_error_code(): # Given spy_hook = MagicMock(spec=Hook)