From c36a6e5d6464739db8bc17896eb8287cbf7e6d68 Mon Sep 17 00:00:00 2001 From: Eduardo Cunha Date: Mon, 23 Jun 2025 16:11:08 +0100 Subject: [PATCH] fix: do not perform flag resolution if not ready The flag resolution was being performed regardles of the state of the client provider. Now, if the provider state is :not_ready or :fatal, it will not attempt to resolve the flag value. Signed-off-by: Eduardo Cunha --- lib/open_feature/client.ex | 68 +++++++++++++++------- lib/open_feature/types.ex | 2 +- test/integration/value_resolution_test.exs | 2 +- test/unit/open_feature/client_test.exs | 40 ++++++++++++- 4 files changed, 86 insertions(+), 26 deletions(-) diff --git a/lib/open_feature/client.ex b/lib/open_feature/client.ex index 93b3327..6b936e3 100644 --- a/lib/open_feature/client.ex +++ b/lib/open_feature/client.ex @@ -82,7 +82,7 @@ defmodule OpenFeature.Client do try do handler.(%{provider: client.provider.name, domain: client.domain}) rescue - e -> Logger.error("Error running event handler. Error: #{e}") + e -> Logger.error("Error running event handler. Error: #{inspect(e)}") end end @@ -260,6 +260,8 @@ defmodule OpenFeature.Client do try do before_hooks(all_hooks, hook_context, opts) + short_circuit_if_not_ready(client) + evaluation_details = evaluation_details(client, type, key, default, merged_context) after_hooks(all_hooks, hook_context, evaluation_details, opts) @@ -267,16 +269,8 @@ defmodule OpenFeature.Client do evaluation_details rescue e -> - Logger.error("An error happened. #{inspect(e)}") error_hooks(all_hooks, hook_context, e, opts) - - %EvaluationDetails{ - key: key, - value: default, - reason: :error, - error_code: :general, - error_message: Exception.message(e) - } + handle_evaluate_error(e, key, default) after all_hooks |> Enum.reverse() @@ -290,6 +284,14 @@ defmodule OpenFeature.Client do |> Map.fetch!(:value) end + defp short_circuit_if_not_ready(%__MODULE__{provider: provider}) do + case Map.get(provider, :state) do + :not_ready -> raise "provider_not_ready" + :fatal -> raise "provider_fatal" + _state -> :ok + end + end + defp evaluation_details(client, type, key, default, context) do details = client @@ -316,24 +318,22 @@ defmodule OpenFeature.Client do error_message: "flag not found" } - {:error, :variant_not_found, _variant} -> + {:error, :variant_not_found, variant} -> %EvaluationDetails{ key: key, value: default, reason: :error, error_code: :general, - error_message: "variant not found" + error_message: "variant not found, variant: #{inspect(variant)}" } {:error, :unexpected_error, error} -> - Logger.error("Unexpected error happened while resolving value. Error: #{error}") - %EvaluationDetails{ key: key, value: default, reason: :error, error_code: :general, - error_message: "unexpected error" + error_message: "unexpected error, error: #{inspect(error)}" } end end @@ -367,9 +367,7 @@ defmodule OpenFeature.Client do try do error_hook.(hook_context, error, hook_hints) rescue - e -> - Logger.error("Unhandled error during 'error' hook: #{inspect(e)}") - Logger.error(inspect(__STACKTRACE__)) + _e -> :ok end end) end @@ -385,10 +383,38 @@ defmodule OpenFeature.Client do try do finally.(hook_context, hook_hints) rescue - e -> - Logger.error("Unhandled error during 'finally' hook: #{inspect(e)}") - Logger.error(inspect(__STACKTRACE__)) + _e -> :ok end end) end + + defp handle_evaluate_error(%RuntimeError{message: "provider_not_ready"}, key, default) do + %EvaluationDetails{ + key: key, + value: default, + reason: :error, + error_code: :provider_not_ready, + error_message: "provider not ready" + } + end + + defp handle_evaluate_error(%RuntimeError{message: "provider_fatal"}, key, default) do + %EvaluationDetails{ + key: key, + value: default, + reason: :error, + error_code: :provider_fatal, + error_message: "provider fatal" + } + end + + defp handle_evaluate_error(e, key, default) do + %EvaluationDetails{ + key: key, + value: default, + reason: :error, + error_code: :general, + error_message: Exception.message(e) + } + end end diff --git a/lib/open_feature/types.ex b/lib/open_feature/types.ex index d619466..a87d521 100644 --- a/lib/open_feature/types.ex +++ b/lib/open_feature/types.ex @@ -19,7 +19,7 @@ defmodule OpenFeature.Types do | :general @type reason :: :static | :default | :targeting_match | :split | :cached | :disabled | :unknown | :stale | :error @type flag_metadata :: %{binary => boolean | binary | number} - @type provider_status :: :not_ready | :ready | :stale + @type provider_status :: :not_ready | :ready | :error | :stale | :fatal @type event_type :: :ready | :error | :configuration_changed | :stale @type event_details :: %{:provider => binary, optional(:domain) => binary, optional(binary | atom) => any} @type event_handler :: (event_details -> any()) diff --git a/test/integration/value_resolution_test.exs b/test/integration/value_resolution_test.exs index 47a9f91..56ee72e 100644 --- a/test/integration/value_resolution_test.exs +++ b/test/integration/value_resolution_test.exs @@ -58,7 +58,7 @@ defmodule Integration.ValueResolutionTest do variant: nil, reason: :error, error_code: :general, - error_message: "variant not found" + error_message: "variant not found, variant: \"variant2\"" } = Client.get_string_details(client, "target_key", "default", context: %{variant: "variant2"}) end diff --git a/test/unit/open_feature/client_test.exs b/test/unit/open_feature/client_test.exs index 1b4197d..2c92778 100644 --- a/test/unit/open_feature/client_test.exs +++ b/test/unit/open_feature/client_test.exs @@ -24,7 +24,7 @@ defmodule OpenFeature.ClientTest do {:ok, client: client} end - describe("Public API") do + describe "Public API" do test "set_context/2 sets the context", %{client: client} do context = %{user: "test_user"} updated_client = Client.set_context(client, context) @@ -439,7 +439,7 @@ defmodule OpenFeature.ClientTest do value: ^default, reason: :error, error_code: :general, - error_message: "variant not found" + error_message: "variant not found, variant: \"variant\"" } = Client.get_boolean_details(client, key, default) end @@ -456,7 +456,41 @@ defmodule OpenFeature.ClientTest do value: ^default, reason: :error, error_code: :general, - error_message: "unexpected error" + error_message: "unexpected error, error: :error" + } = Client.get_boolean_details(client, key, default) + end + + test "returns evaluation details with default value if provider is not ready", %{client: client} do + key = "key" + default = true + + client = put_in(client.provider.state, :not_ready) + + reject(Provider, :resolve_value, 5) + + assert %EvaluationDetails{ + key: ^key, + value: ^default, + reason: :error, + error_code: :provider_not_ready, + error_message: "provider not ready" + } = Client.get_boolean_details(client, key, default) + end + + test "returns evaluation details with default value if provider is fatal", %{client: client} do + key = "key" + default = true + + client = put_in(client.provider.state, :fatal) + + reject(Provider, :resolve_value, 5) + + assert %EvaluationDetails{ + key: ^key, + value: ^default, + reason: :error, + error_code: :provider_fatal, + error_message: "provider fatal" } = Client.get_boolean_details(client, key, default) end end