diff --git a/.changeset/noop-empty-api-key.md b/.changeset/noop-empty-api-key.md new file mode 100644 index 0000000..46216b0 --- /dev/null +++ b/.changeset/noop-empty-api-key.md @@ -0,0 +1,5 @@ +--- +'posthog-ruby': patch +--- + +Initialize disabled no-op clients instead of raising or sending requests when the API key is missing or blank. diff --git a/lib/posthog/client.rb b/lib/posthog/client.rb index 0bd022e..92e4904 100644 --- a/lib/posthog/client.rb +++ b/lib/posthog/client.rb @@ -52,7 +52,7 @@ def _decrement_instance_count(api_key) end # @param opts [Hash] Client configuration. - # @option opts [String] :api_key Your project's API key. Required. + # @option opts [String, nil] :api_key Your project's API key. Missing or blank values disable the client. # @option opts [String, nil] :personal_api_key Your personal API key. Required for local feature flag evaluation. # @option opts [String] :host Fully qualified hostname of the PostHog server. Defaults to `https://us.i.posthog.com`. # @option opts [Integer] :max_queue_size Maximum number of calls to remain queued. Defaults to 10_000. @@ -82,11 +82,12 @@ def initialize(opts = {}) @queue = Queue.new @api_key = opts[:api_key] + @disabled = @api_key.nil? || @api_key.empty? @max_queue_size = opts[:max_queue_size] || Defaults::Queue::MAX_SIZE @worker_mutex = Mutex.new - @sync_mode = opts[:sync_mode] == true && !opts[:test_mode] + @sync_mode = opts[:sync_mode] == true && !opts[:test_mode] && !@disabled @on_error = opts[:on_error] || proc { |status, error| } - @worker = if opts[:test_mode] + @worker = if opts[:test_mode] || @disabled NoopWorker.new(@queue) elsif @sync_mode nil @@ -105,11 +106,10 @@ def initialize(opts = {}) @feature_flags_poller = nil @personal_api_key = opts[:personal_api_key] - check_api_key! - logger.error('api_key is empty after trimming whitespace; check your project API key') if @api_key == '' + logger.error('api_key is missing or empty after trimming whitespace; check your project API key') if @disabled # Warn when multiple clients are created with the same API key (can cause dropped events) - unless opts[:test_mode] || opts[:disable_singleton_warning] + unless @disabled || opts[:test_mode] || opts[:disable_singleton_warning] previous_count = self.class._increment_instance_count(@api_key) if previous_count >= 1 logger.warn( @@ -121,16 +121,18 @@ def initialize(opts = {}) end end - @feature_flags_poller = - FeatureFlagsPoller.new( - opts[:feature_flags_polling_interval], - opts[:personal_api_key], - @api_key, - opts[:host], - opts[:feature_flag_request_timeout_seconds] || Defaults::FeatureFlags::FLAG_REQUEST_TIMEOUT_SECONDS, - opts[:on_error], - flag_definition_cache_provider: opts[:flag_definition_cache_provider] - ) + unless @disabled + @feature_flags_poller = + FeatureFlagsPoller.new( + opts[:feature_flags_polling_interval], + opts[:personal_api_key], + @api_key, + opts[:host], + opts[:feature_flag_request_timeout_seconds] || Defaults::FeatureFlags::FLAG_REQUEST_TIMEOUT_SECONDS, + opts[:on_error], + flag_definition_cache_provider: opts[:flag_definition_cache_provider] + ) + end @distinct_id_has_sent_flag_calls = SizeLimitedHash.new(Defaults::MAX_HASH_SIZE) do |hash, key| hash[key] = [] @@ -198,6 +200,8 @@ def clear # @return [Boolean] Whether the event was queued or sent. # @macro common_attrs def capture(attrs) + return false if @disabled + symbolize_keys! attrs enrich_capture_attrs_with_context(attrs) @@ -226,7 +230,7 @@ def capture(attrs) end send_feature_flags_param = attrs[:send_feature_flags] - if send_feature_flags_param + if send_feature_flags_param && !@disabled _emit_deprecation( :capture_send_feature_flags, '`send_feature_flags` on `capture` is deprecated and will be removed in a future major ' \ @@ -388,6 +392,8 @@ def is_feature_enabled( # rubocop:disable Naming/PredicateName # @param flag_key [String, Symbol] The unique flag key of the remote config feature flag. # @return [Hash] The parsed remote config payload response. def get_remote_config_payload(flag_key) + return nil if @disabled + @feature_flags_poller.get_remote_config_payload(flag_key.to_s) end @@ -465,6 +471,8 @@ def get_feature_flag_result( '`flags.get_flag_payload(key)` instead — this consolidates flag evaluation into a single ' \ '`/flags` request per incoming request.' ) + return nil if @disabled + _get_feature_flag_result( key, distinct_id, groups: groups, person_properties: person_properties, group_properties: group_properties, @@ -507,6 +515,8 @@ def evaluate_flags( return FeatureFlagEvaluations.new(host: host, distinct_id: '', flags: {}) end + return FeatureFlagEvaluations.new(host: host, distinct_id: distinct_id, flags: {}, groups: groups) if @disabled + person_properties, group_properties = add_local_person_and_group_properties( distinct_id, groups, person_properties, group_properties ) @@ -620,6 +630,8 @@ def get_all_flags( group_properties: {}, only_evaluate_locally: false ) + return {} if @disabled + person_properties, group_properties = add_local_person_and_group_properties(distinct_id, groups, person_properties, group_properties) @feature_flags_poller.get_all_flags(distinct_id, groups, person_properties, group_properties, @@ -657,6 +669,8 @@ def get_feature_flag_payload( 'instead — this consolidates flag evaluation into a single `/flags` request per ' \ 'incoming request.' ) + return nil if @disabled + key = key.to_s person_properties, group_properties = add_local_person_and_group_properties(distinct_id, groups, person_properties, group_properties) @@ -683,6 +697,8 @@ def get_all_flags_and_payloads( group_properties: {}, only_evaluate_locally: false ) + return { featureFlags: {}, featureFlagPayloads: {} } if @disabled + person_properties, group_properties = add_local_person_and_group_properties( distinct_id, groups, person_properties, group_properties ) @@ -700,6 +716,8 @@ def get_all_flags_and_payloads( # # @return [void] def reload_feature_flags + return if @disabled + unless @personal_api_key logger.error( 'You need to specify a personal_api_key to locally evaluate feature flags' @@ -713,8 +731,8 @@ def reload_feature_flags # # @return [void] def shutdown - self.class._decrement_instance_count(@api_key) if @api_key - @feature_flags_poller.shutdown_poller + self.class._decrement_instance_count(@api_key) unless @disabled + @feature_flags_poller&.shutdown_poller flush if @sync_mode @sync_lock.synchronize { @transport&.shutdown } @@ -810,6 +828,8 @@ def _get_feature_flag_result( only_evaluate_locally: false, send_feature_flag_events: true ) + return nil if @disabled + key = key.to_s person_properties, group_properties = add_local_person_and_group_properties( distinct_id, groups, person_properties, group_properties @@ -875,6 +895,8 @@ def process_before_send(action) # # returns Boolean of whether the item was added to the queue. def enqueue(action) + return false if @disabled + action = process_before_send(action) return false if action.nil? || action.empty? @@ -901,11 +923,6 @@ def enqueue(action) end end - # private: Checks that the api_key is properly initialized - def check_api_key! - raise ArgumentError, 'API key must be initialized' if @api_key.nil? - end - def normalize_string_option(value, blank_as_nil: false) return value unless value.is_a?(String) diff --git a/spec/posthog/client_spec.rb b/spec/posthog/client_spec.rb index 60692f5..d4f2237 100644 --- a/spec/posthog/client_spec.rb +++ b/spec/posthog/client_spec.rb @@ -21,8 +21,52 @@ module PostHog end describe '#initialize' do - it 'errors if no api_key is supplied' do - expect { Client.new }.to raise_error(ArgumentError) + it 'creates a disabled client if no api_key is supplied' do + client = nil + + expect { client = Client.new }.to_not raise_error + + expect(client.instance_variable_get(:@disabled)).to eq(true) + expect(client.instance_variable_get(:@worker)).to be_a(PostHog::NoopWorker) + end + + shared_examples 'a disabled client for an invalid api_key' do |api_key| + it 'creates a disabled client' do + client = Client.new(api_key: api_key) + + expect(client.instance_variable_get(:@disabled)).to eq(true) + expect(client.instance_variable_get(:@worker)).to be_a(PostHog::NoopWorker) + expect(FieldParser).not_to receive(:parse_for_capture) + expect(client.capture(Queued::CAPTURE)).to eq(false) + expect(client.queued_messages).to eq(0) + end + + it 'does not start a sender or sync transport' do + expect(PostHog::SendWorker).not_to receive(:new) + expect(PostHog::Transport).not_to receive(:new) + + Client.new(api_key: api_key, sync_mode: true) + end + + it 'logs that the api_key is missing or empty after trimming whitespace' do + Client.new(api_key: api_key, test_mode: true) + + expect(logger).to have_received(:error) + .with(include('api_key is missing or empty after trimming whitespace')) + .once + end + end + + context 'when api_key is nil' do + include_examples 'a disabled client for an invalid api_key', nil + end + + context 'when api_key is empty' do + include_examples 'a disabled client for an invalid api_key', '' + end + + context 'when api_key is blank after trimming' do + include_examples 'a disabled client for an invalid api_key', " \n\t " end it 'does not error if a api_key is supplied' do @@ -58,12 +102,6 @@ module PostHog expect(client.instance_variable_get(:@feature_flags_poller).instance_variable_get(:@host)).to eq('https://us.i.posthog.com') end - it 'logs when the api_key is empty after trimming whitespace' do - Client.new(api_key: " \n\t ", test_mode: true) - - expect(logger).to have_received(:error).with(include('api_key is empty after trimming whitespace')) - end - context 'singleton warning' do before do # Stub HTTP to allow creating clients without test_mode (which triggers the warning) @@ -1379,6 +1417,29 @@ def run end describe 'feature flags' do + it 'returns defaults without requests when the client is disabled' do + disabled_client = Client.new(api_key: " \n\t ", test_mode: true) + + expect(disabled_client.instance_variable_get(:@feature_flags_poller)).to be_nil + expect(disabled_client.get_remote_config_payload('remote-config')).to be_nil + expect(disabled_client.get_feature_flag('flag', 'some id')).to be_nil + expect(disabled_client.get_feature_flag_result('flag', 'some id')).to be_nil + expect(disabled_client.get_feature_flag_payload('flag', 'some id')).to be_nil + expect(disabled_client.get_all_flags('some id')).to eq({}) + expect(disabled_client.get_all_flags_and_payloads('some id')).to eq( + { featureFlags: {}, featureFlagPayloads: {} } + ) + + flags = disabled_client.evaluate_flags('some id') + expect(flags.keys).to eq([]) + expect(flags.enabled?('flag')).to eq(false) + expect(flags.get_flag('flag')).to be_nil + expect(flags.get_flag_payload('flag')).to be_nil + + expect(WebMock).not_to have_requested(:get, /.*/) + expect(WebMock).not_to have_requested(:post, /.*/) + end + it 'evaluates flags correctly' do api_feature_flag_res = { flags: [