PostHog / posthog-js-lite

Reimplementation of posthog-js to be as light and modular as possible.
https://posthog.com/docs/libraries
MIT License
51 stars 27 forks source link

fix(flags): Add a shorter timeout, disable retries for flags #197

Closed neilkakkar closed 4 months ago

neilkakkar commented 4 months ago

Problem

^^ We want a much shorter timeout on flags, since they can block client applications. Also makes this configurable now.

Fixes https://github.com/PostHog/posthog-js-lite/issues/195

Changes

Release info Sub-libraries affected

Bump level

Libraries affected

Changelog notes

github-actions[bot] commented 4 months ago

Size Change: 0 B 🆕

Total Size: 0 B

compressed-size-action

marandaneto commented 4 months ago

@neilkakkar the original issue is about posthog.getAllFlags() which forwards the call to getAllFlagsAndPayloads, then _loadFeatureFlags. _loadFeatureFlags seems to have its own timeout which is 10s, should the api/feature_flag endpoint share the decide timeout or is it something else?

marandaneto commented 4 months ago

I'm wondering how the signal abort timeout is computed, didn't find anything here https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static The timeout must be just idle IMO, instead of the whole thing, for example, connect, DNS handshake, waiting for API request, downloading bytes, answering, etc, 3s would be very low for slow network/emerging countries as a default, consider also a proxy/vpn slowdowns.

neilkakkar commented 4 months ago

The loadFeatureFlags timeout is for local evaluation flags which are cached, so can have a much longer timeout. I don't think that's where the problem came in here, it's after, when you call decide/ + retry.

The default timeout for all requests works for local eval as well, since generally it needs to succeed only once, rather than everytime we call getFeatureFlag or getAllFlags()

marandaneto commented 4 months ago

@neilkakkar once you address https://github.com/PostHog/posthog-js-lite/pull/197#discussion_r1510286304 we can merge and cut the first preview of the new major version of the SDKs