fix: Disable SDK when API key is blank#124
Conversation
posthog-elixir Compliance ReportDate: 2026-05-29 09:10:18 UTC
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 609ms |
| Format Validation.Event Has Uuid | ✅ | 610ms |
| Format Validation.Event Has Lib Properties | ✅ | 610ms |
| Format Validation.Distinct Id Is String | ✅ | 610ms |
| Format Validation.Token Is Present | ✅ | 610ms |
| Format Validation.Custom Properties Preserved | ✅ | 610ms |
| Format Validation.Event Has Timestamp | ✅ | 610ms |
| Retry Behavior.Retries On 503 | ✅ | 5615ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2612ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2612ms |
| Retry Behavior.Respects Retry After Header | ✅ | 5616ms |
| Retry Behavior.Implements Backoff | ✅ | 15626ms |
| Retry Behavior.Retries On 500 | ✅ | 5614ms |
| Retry Behavior.Retries On 502 | ✅ | 5614ms |
| Retry Behavior.Retries On 504 | ✅ | 5615ms |
| Retry Behavior.Max Retries Respected | ✅ | 15625ms |
| Deduplication.Generates Unique Uuids | ✅ | 621ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5615ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10622ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5617ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 615ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 611ms |
| Compression.Sends Gzip When Enabled | ✅ | 610ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 610ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 607ms |
| Batch Format.Multiple Events Batched Together | ✅ | 615ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2612ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2611ms |
| Error Handling.Retries On 408 | ✅ | 5615ms |
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 9ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 6ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 6ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 7ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 6ms |
| Request Payload.Groups Round Trip | ✅ | 6ms |
| Request Payload.Groups Default To Empty Object | ✅ | 7ms |
| Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It | ✅ | 6ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 7ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 6ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 6ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 3ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 609ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 11ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ❌ | 6ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ❌ | 609ms |
Failures
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['groups', 'distinct_id', 'api_key', 'flag_keys_to_evaluate', 'group_properties', 'person_properties']
request_lifecycle.mock_response_value_is_returned_to_caller
Last action result missing field 'value'. Keys: ['error', 'success']
side_effect_events.get_feature_flag_captures_feature_flag_called_event
Expected 1 events with name '$feature_flag_called', got 0
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
test/posthog/config_test.exs:63-134
**Prefer parameterised tests for the four blank-key cases**
The four tests (`missing`, `nil`, `empty`, `blank after trimming whitespace`) exercise exactly the same assertions with only the input value changing. Per the team's convention, these should be a single parameterised test. The same applies to the two `read!` tests in `application_config_test.exs` (`missing` vs `nil`). A for-comprehension over a list of `{label, input}` pairs keeps the intent clear and removes the duplication.
### Issue 2 of 2
lib/posthog/config.ex:258-266
The third `then/2` block injects `api_host: @default_api_host` when `api_key` is blank and `api_host` is absent from the keyword list. However, the NimbleOptions schema already declares `default: @default_api_host` for `api_host`, so NimbleOptions itself will supply that default during validation — the explicit injection is redundant and adds a code path that has no observable effect.
```suggestion
end
```
Reviews (1): Last reviewed commit: "Disable SDK when API key is blank" | Re-trigger Greptile |
| [] -> | ||
| :ok |
There was a problem hiding this comment.
I think this will prevent test mode from working if api key isn't set. Is it ok? Setting fake key is not a big deal, just not a very obvious configuration options interaction
There was a problem hiding this comment.
I'd expect tests to just set up a fake API tbh
There was a problem hiding this comment.
You mean that users will just set it up? OK yeah, that works.
There was a problem hiding this comment.
yeah otherwise we have to bypass checks just for test mode
Tests should verify an almost real-life scenario, so a non-empty API key is the bare minimum.
|
can't approve, but LGTM |
💡 Motivation and Context
Applications that omit or blank out the PostHog API key should be able to boot with the SDK disabled instead of raising during configuration or attempting to send events.
This change treats missing,
nil, empty, or whitespace-only API keys as disabled/no-op configuration while still allowing the application supervision tree to start safely. It skips sender/source workers when disabled, lets captures no-op when there are no sender workers, and makes feature flag calls return empty flag results instead of attempting to use a nil API client.💚 How did you test it?
mix test test/posthog/feature_flags_test.exs test/posthog/feature_flags/evaluations_test.exsmix test test/posthog/supervisor_test.exs test/posthog/sender_test.exsmix testmix format --check-formatted📝 Checklist
If releasing new changes
sampo addto generate a changeset file