Do not push to the loaded channel when flags' request fails

[zaynetro]

If we get HTTP timeouts or server errors then this SDK will happily block itself.

In case of an error poller.loaded <- false will block indefinitely since we read from this channel only once. Blocking on the channel will in turn block the infinite loop that polls for the latest state. SDK users are not blocked but will get outdated flags' state.

Tests before:

> go test -run TestFetchFlagsFails -timeout 5s

panic: test timed out after 5s
running tests:
    TestFetchFlagsFails (5s)

...

goroutine 35 [chan send]:
github.com/posthog/posthog-go.(*FeatureFlagsPoller).ForceReload(...)
    /Users/roman/Code/trial/posthog-go/featureflags.go:874
github.com/posthog/posthog-go.(*client).ReloadFeatureFlags(0x140001a40d8?)
    /Users/roman/Code/trial/posthog-go/posthog.go:272 +0x34
github.com/posthog/posthog-go.TestFetchFlagsFails(0x140001a8680)
    /Users/roman/Code/trial/posthog-go/feature_flags_test.go:3416 +0x1ac
testing.tRunner(0x140001a8680, 0x104706608)
    /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:1689 +0xec
created by testing.(*T).Run in goroutine 1
    /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:1742 +0x318

goroutine 37 [chan send]:
github.com/posthog/posthog-go.(*FeatureFlagsPoller).fetchNewFeatureFlags(0x140002061c0)
    /Users/roman/Code/trial/posthog-go/featureflags.go:179 +0x124
github.com/posthog/posthog-go.(*FeatureFlagsPoller).run(0x140002061c0)
    /Users/roman/Code/trial/posthog-go/featureflags.go:166 +0xac
created by github.com/posthog/posthog-go.newFeatureFlagsPoller in goroutine 35
    /Users/roman/Code/trial/posthog-go/featureflags.go:148 +0x1e8

...

Here you can see that we are waiting on

github.com/posthog/posthog-go.(*FeatureFlagsPoller).fetchNewFeatureFlags(0x140002061c0)
    /Users/roman/Code/trial/posthog-go/featureflags.go:179 +0x124

which is poller.loaded <- false

[neilkakkar]

This is great, thanks @zaynetro ! A bit unsure of the go version update on the main package, will that break existing clients on v1.18? Given they might not have a pinned version of posthog-go. Not really familiar with go package management yet.

Can we use a "polyfill" of sync/atomic for that specific test?

[zaynetro]

Go 1.19 was released on 2022-08-02. Since then there were three major Go releases 1.20, 1.21 and 1.22. It is rather old and no longer supported version.

Since sync/atomic is used only in the test it looks like go 1.18 will continue to build posthog-go module but will fail to run tests.

% /nix/store/lmddll9adkay4qgdphgjz77ydik73gj3-go-1.18.10/bin/go version
go version go1.18.10 darwin/arm64

% /nix/store/lmddll9adkay4qgdphgjz77ydik73gj3-go-1.18.10/bin/go build
% echo $?
0

% /nix/store/lmddll9adkay4qgdphgjz77ydik73gj3-go-1.18.10/bin/go test 
# github.com/posthog/posthog-go [github.com/posthog/posthog-go.test]
./feature_flags_test.go:3391:20: undefined: atomic.Uint32
note: module requires Go 1.19
FAIL    github.com/posthog/posthog-go [build failed]

[zaynetro]

@neilkakkar what do you think? Do you still want me to revert Go version update?

[neilkakkar]

Hey @zaynetro , been trying to find if we have any clients on v1.18 , but we don't really track this information so hard to say what the impact will be, specially since this Go SDK doesn't yet have proper minor version - versioning.

As such, would prefer minimising chance of things going wrong and reverting the version update change.

[zaynetro]

ok rolled back the change of Go compiler version

[neilkakkar]

Sorry for the low responsiveness here, thursday's incident + review has me busy 😅 - will merge this monday!

[zaynetro]

@neilkakkar just a friendly reminder

[neilkakkar]

Thank you, this is great :)