Improve error handling when PostHog is disabled in config

[urcadox]

Hi there,

When PostHog is disabled in the config, calling PostHog.capture/2 raises an ArgumentError with the following message:

[error] ** (ArgumentError) unknown registry: PostHog.Registry. Either the registry name is invalid or the registry is not running, possibly because its application isn't started

This forces users to wrap calls to PostHog.capture/2 to handle this case.

In my opinion, it should return something like {:error, :posthog_disabled} which is easier to ignore (and you can always use :ok = PostHog.capture(...) if you want).

Thoughts?

[martosaur]

Hmm, I think this makes sense. May I ask about your use case? Just want to better understand the scenario when posthog is disabled but there are calls to capture in the codebase.

[urcadox]

Alright, here's an example.

I added event tracking in one of $job's apps; for development I created a project in a personal organization.

For production, I created another project in $job's organization.

Creating a dev project in there feels like unnecessary noise to me, and I don't want every dev to have to bother creating a PostHog project in their own organizations (nor having to set an API key for something that does not have any use most of the time). So disabling it by default in dev mode seems like the best thing to do in my opinion.

I haven't tested this but maybe I'm supposed to keep PostHog enabled without a valid API key and it would just fail silently when actually trying to send the events? (That does not feel right to me though)

[martosaur]

Yeah I agree disabling events capture in dev is totally reasonable and is probably pretty common. I didn't really thought about it because our team (I'm not a posthog employee, just care a lot about sdk) just created a dev project and it's shared among all engineers.

Thanks for bringing it up, I'll look into it! Unless someone else do it first of course😆

[rafaeelaudibert]

Thanks for the report! - and thanks for chiming in @martosaur.

Yeah, this changes a little bit how this is implemented because setting enable: false simply means we don't start our supervisor - which you've probably figured out by the error message.

It's not clear to me where we should fail the request right now. We can do that inside capture to return an early failure but that opens us to bugs in the future where we forget to check new code paths hitting the servers because this isn't centralized. It also means we might fire requests for something like flags since the codepaths are now separate.

Another alternative is to add that inside something like the PostHog.Sender class. That's positive because we don't need to check the config as often (which might be costly since it's stored in a registry) but we keep having the problem of having to reimplement the if enabled check in many different places (again, like flags).

[martosaur]

After looking more into this, I think the best way to support "disabling" sdk in dev, would be to have a configuration option to switch off sending events.

Just handling the exception is not the right path IMHO, because it won't work if you prefer assertive style of writing :ok = PostHog.capture. I also feel it's kind of bad to not run a supervision subtree in dev that you plan to run in prod. One day something will blow up and you might discover it too late!

I'd propose we add a configuration option send_events: false that would simply instruct PostHog.Sender to drop events and not send them anywhere. It kind of sucks that I made test_mode a boolean, since it would fit very well as a separate mode, e.g. mode :: :test | :drop_events | :normal or something like this. Maybe it's not too late to add mode and deprecate test_mode 😆 I'll leave hard decisions to @rafaeelaudibert

There is also a great question about feature-flags related functions raised by @rafaeelaudibert . It looks to me they are indifferent to this either way. These functions exist for their return, so users will have to handle error tuple either way. I can see why somebody would want to use real feature flags in dev, but don't send events, so I would not guard them behind the same switch as events. As long as they don't raise, it should be fine!