diff --git a/.sampo/changesets/noop-empty-api-key.md b/.sampo/changesets/noop-empty-api-key.md new file mode 100644 index 0000000..500efdf --- /dev/null +++ b/.sampo/changesets/noop-empty-api-key.md @@ -0,0 +1,5 @@ +--- +hex/posthog: patch +--- + +Start in disabled/no-op mode instead of raising or sending events when the API key is missing, blank, or the supervisor is unavailable. diff --git a/lib/posthog/application.ex b/lib/posthog/application.ex index 23b6d24..238f1a0 100644 --- a/lib/posthog/application.ex +++ b/lib/posthog/application.ex @@ -8,7 +8,7 @@ defmodule PostHog.Application do children = if conv_config.enable do - if conv_config.enable_error_tracking do + if conv_config.enable_error_tracking and supervisor_config.enabled do :logger.add_handler(:posthog, PostHog.Handler, %{config: supervisor_config}) end diff --git a/lib/posthog/config.ex b/lib/posthog/config.ex index d6da100..8bc5ccd 100644 --- a/lib/posthog/config.ex +++ b/lib/posthog/config.ex @@ -21,9 +21,10 @@ defmodule PostHog.Config do ], api_key: [ type: :string, - required: true, + default: "", doc: """ Your PostHog Project API key. Find it in your project's settings under the Project ID section. + If omitted or empty after trimming whitespace, PostHog starts in disabled/no-op mode. """ ], api_client_module: [ @@ -210,15 +211,24 @@ defmodule PostHog.Config do with {:ok, validated} <- NimbleOptions.validate(normalized_options, @compiled_configuration_schema) do + api_key_blank? = blank_api_key?(validated) log_blank_api_key(validated) config = Map.new(validated) - client = config.api_client_module.client(config.api_key, config.api_host) + + client = + if api_key_blank? do + nil + else + config.api_client_module.client(config.api_key, config.api_host) + end + global_properties = Map.merge(config.global_properties, @system_global_properties) final_config = config |> Map.put(:api_client, client) + |> Map.put(:enabled, not api_key_blank?) |> Map.put( :in_app_modules, config.in_app_otp_apps |> Enum.flat_map(&Application.spec(&1, :modules)) |> MapSet.new() @@ -248,6 +258,7 @@ defmodule PostHog.Config do end defp normalize_api_key(api_key) when is_binary(api_key), do: String.trim(api_key) + defp normalize_api_key(nil), do: "" defp normalize_api_key(api_key), do: api_key defp normalize_api_host(api_host) when is_binary(api_host) do @@ -261,10 +272,12 @@ defmodule PostHog.Config do defp normalize_api_host(api_host), do: api_host + defp blank_api_key?(validated), do: validated[:api_key] == "" + defp log_blank_api_key(validated) do - if validated[:api_key] == "" do - Logger.error( - "posthog api_key is empty after trimming whitespace; check your project API key" + if blank_api_key?(validated) do + Logger.warning( + "posthog api_key is empty after trimming whitespace; PostHog will start in disabled/no-op mode" ) end end diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index d6e5e36..3e97cad 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -29,7 +29,15 @@ defmodule PostHog.FeatureFlags do def flags(name \\ PostHog, body) do config = PostHog.config(name) - case PostHog.API.flags(config.api_client, body) do + if Map.get(config, :enabled, true) do + request_flags(config.api_client, body) + else + empty_flags_response() + end + end + + defp request_flags(api_client, body) do + case PostHog.API.flags(api_client, body) do {:ok, %{status: 200, body: %{"flags" => _}}} = resp -> resp @@ -48,6 +56,8 @@ defmodule PostHog.FeatureFlags do end end + defp empty_flags_response, do: {:ok, %{status: 200, body: %{"flags" => %{}}}} + @doc false def flags_for(distinct_id_or_body) when not is_atom(distinct_id_or_body), do: flags_for(PostHog, distinct_id_or_body) diff --git a/lib/posthog/registry.ex b/lib/posthog/registry.ex index 8115116..0a787ba 100644 --- a/lib/posthog/registry.ex +++ b/lib/posthog/registry.ex @@ -1,12 +1,28 @@ defmodule PostHog.Registry do @moduledoc false def config(supervisor_name) do - {:ok, config} = - supervisor_name - |> registry_name() - |> Registry.meta(:config) + registry = registry_name(supervisor_name) - config + if Process.whereis(registry) do + case Registry.meta(registry, :config) do + {:ok, config} -> config + :error -> disabled_config(supervisor_name) + end + else + disabled_config(supervisor_name) + end + rescue + ArgumentError -> disabled_config(supervisor_name) + end + + def disabled_config(supervisor_name) do + %{ + supervisor_name: supervisor_name, + enabled: false, + api_client: nil, + global_properties: %{}, + test_mode: false + } end def registry_name(supervisor_name), do: Module.concat(supervisor_name, Registry) diff --git a/lib/posthog/sender.ex b/lib/posthog/sender.ex index 2c3ccfe..0970fa3 100644 --- a/lib/posthog/sender.ex +++ b/lib/posthog/sender.ex @@ -28,27 +28,44 @@ defmodule PostHog.Sender do # Client def send(event, supervisor_name) do - supervisor_name - |> PostHog.Registry.config() - |> case do - %{test_mode: true} -> - PostHog.Test.remember_event(supervisor_name, event) - - _ -> - senders = - supervisor_name - |> PostHog.Registry.registry_name() - |> Registry.select([{{{__MODULE__, :_}, :"$1", :"$2"}, [], [{{:"$2", :"$1"}}]}]) - - # Pick the first available sender, otherwise random busy one. - senders - |> Keyword.get_lazy(:available, fn -> - senders |> Keyword.values() |> Enum.random() - end) - |> GenServer.cast({:event, event}) + case senders(supervisor_name) do + [] -> + :ok + + senders -> + supervisor_name + |> PostHog.Registry.config() + |> case do + %{test_mode: true} -> + PostHog.Test.remember_event(supervisor_name, event) + + _ -> + send_to_sender(event, senders) + end end end + defp senders(supervisor_name) do + registry = PostHog.Registry.registry_name(supervisor_name) + + if Process.whereis(registry) do + Registry.select(registry, [{{{__MODULE__, :_}, :"$1", :"$2"}, [], [{{:"$2", :"$1"}}]}]) + else + [] + end + rescue + ArgumentError -> [] + end + + defp send_to_sender(event, senders) do + # Pick the first available sender, otherwise random busy one. + senders + |> Keyword.get_lazy(:available, fn -> + senders |> Keyword.values() |> Enum.random() + end) + |> GenServer.cast({:event, event}) + end + # Callbacks @impl GenServer diff --git a/lib/posthog/supervisor.ex b/lib/posthog/supervisor.ex index 61e2a79..9a50539 100644 --- a/lib/posthog/supervisor.ex +++ b/lib/posthog/supervisor.ex @@ -65,6 +65,8 @@ defmodule PostHog.Supervisor do Supervisor.init(children, strategy: :one_for_one) end + defp sources(%{enabled: false}), do: [] + defp sources(config) do if config.enable_source_code_context do opts = [ @@ -81,6 +83,8 @@ defmodule PostHog.Supervisor do end end + defp senders(%{enabled: false}), do: [] + defp senders(config) do pool_size = Map.get(config, :sender_pool_size, max(System.schedulers_online(), 2)) diff --git a/test/posthog/application_config_test.exs b/test/posthog/application_config_test.exs new file mode 100644 index 0000000..40f08fd --- /dev/null +++ b/test/posthog/application_config_test.exs @@ -0,0 +1,50 @@ +defmodule PostHog.ApplicationConfigTest do + use ExUnit.Case, async: false + + import ExUnit.CaptureLog + import Mox + + setup :verify_on_exit! + + setup do + previous_env = Application.get_all_env(:posthog) + + on_exit(fn -> + :posthog + |> Application.get_all_env() + |> Keyword.keys() + |> Enum.each(&Application.delete_env(:posthog, &1)) + + Enum.each(previous_env, fn {key, value} -> + Application.put_env(:posthog, key, value) + end) + end) + end + + for {label, api_key_env} <- [ + {"missing", :missing}, + {"nil", nil} + ] do + test "read! does not raise when enabled and api_key is #{label}" do + Application.put_env(:posthog, :enable, true) + Application.put_env(:posthog, :api_client_module, PostHog.API.Mock) + Application.delete_env(:posthog, :api_host) + + case unquote(Macro.escape(api_key_env)) do + :missing -> Application.delete_env(:posthog, :api_key) + api_key -> Application.put_env(:posthog, :api_key, api_key) + end + + log = + capture_log(fn -> + assert {%{enable: true}, %{api_key: "", enabled: false, api_client: nil} = config} = + PostHog.Config.read!() + + assert config.api_host == "https://us.i.posthog.com" + end) + + assert log =~ + "posthog api_key is empty after trimming whitespace; PostHog will start in disabled/no-op mode" + end + end +end diff --git a/test/posthog/config_test.exs b/test/posthog/config_test.exs index 2047d30..0a82278 100644 --- a/test/posthog/config_test.exs +++ b/test/posthog/config_test.exs @@ -60,26 +60,27 @@ defmodule PostHog.ConfigTest do assert config.api_host == "https://us.i.posthog.com" end - test "validate logs when api_key is blank after trimming whitespace" do - expect(PostHog.API.Mock, :client, fn api_key, api_host -> - assert api_key == "" - assert api_host == "https://us.i.posthog.com" - - %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock} - end) - - log = - capture_log(fn -> - assert {:ok, config} = - PostHog.Config.validate( - api_key: " \n\t ", - api_host: "https://us.i.posthog.com", - api_client_module: PostHog.API.Mock - ) - - assert config.api_key == "" - end) - - assert log =~ "posthog api_key is empty after trimming whitespace; check your project API key" + for {label, api_key_option} <- [ + {"missing", []}, + {"nil", [api_key: nil]}, + {"empty", [api_key: ""]}, + {"blank after trimming whitespace", [api_key: " \n\t "]} + ] do + test "validate disables PostHog when api_key is #{label}" do + log = + capture_log(fn -> + options = unquote(Macro.escape(api_key_option)) ++ [api_client_module: PostHog.API.Mock] + + assert {:ok, config} = PostHog.Config.validate(options) + + assert config.api_key == "" + assert config.api_host == "https://us.i.posthog.com" + assert config.enabled == false + assert config.api_client == nil + end) + + assert log =~ + "posthog api_key is empty after trimming whitespace; PostHog will start in disabled/no-op mode" + end end end diff --git a/test/posthog/feature_flags/evaluations_test.exs b/test/posthog/feature_flags/evaluations_test.exs index 8adffc2..406631f 100644 --- a/test/posthog/feature_flags/evaluations_test.exs +++ b/test/posthog/feature_flags/evaluations_test.exs @@ -126,6 +126,12 @@ defmodule PostHog.FeatureFlags.EvaluationsTest do assert all_captured() == [] end + @tag config: [api_key: "", supervisor_name: PostHog] + test "returns an empty snapshot when PostHog is disabled" do + assert {:ok, %Evaluations{distinct_id: "foo", flags: %{}}} = + FeatureFlags.evaluate_flags("foo") + end + @tag config: [supervisor_name: MyPostHog] test "supports a named PostHog instance" do expect(API.Mock, :request, fn _client, :post, "/flags", _opts -> diff --git a/test/posthog/feature_flags_test.exs b/test/posthog/feature_flags_test.exs index ea96673..807e9b3 100644 --- a/test/posthog/feature_flags_test.exs +++ b/test/posthog/feature_flags_test.exs @@ -92,6 +92,12 @@ defmodule PostHog.FeatureFlagsTest do assert {:ok, %{body: %{"flags" => _}}} = FeatureFlags.flags(MyPostHog, %{distinct_id: "foo"}) end + + @tag config: [api_key: "", supervisor_name: PostHog] + test "returns empty flags when PostHog is disabled" do + assert {:ok, %{status: 200, body: %{"flags" => %{}}}} = + FeatureFlags.flags(%{distinct_id: "foo"}) + end end describe "flags_for/2" do diff --git a/test/posthog/supervisor_test.exs b/test/posthog/supervisor_test.exs new file mode 100644 index 0000000..9ec744f --- /dev/null +++ b/test/posthog/supervisor_test.exs @@ -0,0 +1,35 @@ +defmodule PostHog.SupervisorTest do + use ExUnit.Case, async: true + + import ExUnit.CaptureLog + + @supervisor_name __MODULE__ + + test "disabled config starts without senders and captures as no-op" do + {config, _log} = + with_log(fn -> + PostHog.Config.validate!( + api_key: " \n\t ", + api_host: "https://us.i.posthog.com", + supervisor_name: @supervisor_name, + enable_source_code_context: true, + test_mode: true + ) + end) + + start_link_supervised!({PostHog.Supervisor, config}) + + assert [] = + @supervisor_name + |> PostHog.Registry.registry_name() + |> Registry.select([{{{PostHog.Sender, :_}, :"$1", :"$2"}, [], [:"$1"]}]) + + assert nil == + @supervisor_name + |> PostHog.Registry.via(PostHog.ErrorTracking.Sources) + |> GenServer.whereis() + + assert :ok = PostHog.bare_capture(@supervisor_name, "disabled capture", "distinct_id", %{}) + assert PostHog.Test.all_captured(@supervisor_name) == [] + end +end diff --git a/test/posthog/uninitialized_test.exs b/test/posthog/uninitialized_test.exs new file mode 100644 index 0000000..f231f2a --- /dev/null +++ b/test/posthog/uninitialized_test.exs @@ -0,0 +1,21 @@ +defmodule PostHog.UninitializedTest do + use ExUnit.Case, async: true + + alias PostHog.FeatureFlags + alias PostHog.FeatureFlags.Evaluations + + @supervisor_name __MODULE__.MissingSupervisor + + test "public APIs no-op when the supervisor was not started" do + assert %{enabled: false, supervisor_name: @supervisor_name} = PostHog.config(@supervisor_name) + assert :ok = PostHog.bare_capture(@supervisor_name, "event", "distinct_id", %{}) + + PostHog.set_context(@supervisor_name, %{distinct_id: "distinct_id"}) + assert :ok = PostHog.capture(@supervisor_name, "event", %{}) + + assert {:ok, %{}} = FeatureFlags.flags_for(@supervisor_name, "distinct_id") + assert {:ok, snapshot} = FeatureFlags.evaluate_flags(@supervisor_name, "distinct_id") + assert Evaluations.keys(snapshot) == [] + assert Evaluations.enabled?(snapshot, "flag") == false + end +end