diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 1bddd8a..6a2f55b 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -20,4 +20,4 @@ jobs: run: go build . - name: Test - run: go test -v . \ No newline at end of file + run: go test -timeout 30s -v . \ No newline at end of file diff --git a/config.go b/config.go index dd0740c..4624d69 100644 --- a/config.go +++ b/config.go @@ -27,10 +27,13 @@ type Config struct { // timer triggers. Interval time.Duration - // Interval at which to fetch new feature flags, 5min by default + // Interval at which to fetch new feature flag definitions, 5min by default DefaultFeatureFlagsPollingInterval time.Duration - // Calculate when feature flags should be polled next. Setting this property + // Timeout for fetching feature flags, 3 seconds by default + FeatureFlagRequestTimeout time.Duration + + // Calculate when feature flag definitions should be polled next. Setting this property // will override DefaultFeatureFlagsPollingInterval. NextFeatureFlagsPollingTick func() time.Duration @@ -98,6 +101,9 @@ const DefaultInterval = 5 * time.Second // Specifies the default interval at which to fetch new feature flags const DefaultFeatureFlagsPollingInterval = 5 * time.Minute +// Specifies the default timeout for fetching feature flags +const DefaultFeatureFlagRequestTimeout = 3 * time.Second + // This constant sets the default batch size used by client instances if none // was explicitly set. const DefaultBatchSize = 250 @@ -139,6 +145,10 @@ func makeConfig(c Config) Config { c.DefaultFeatureFlagsPollingInterval = DefaultFeatureFlagsPollingInterval } + if c.FeatureFlagRequestTimeout == 0 { + c.FeatureFlagRequestTimeout = DefaultFeatureFlagRequestTimeout + } + if c.Transport == nil { c.Transport = http.DefaultTransport } diff --git a/examples/capture.go b/examples/capture.go index d5bc303..9b46913 100644 --- a/examples/capture.go +++ b/examples/capture.go @@ -2,8 +2,9 @@ package main import ( "fmt" - "github.com/posthog/posthog-go" "time" + + "github.com/posthog/posthog-go" ) func TestCapture() { diff --git a/examples/featureflags.go b/examples/featureflags.go index 355319f..72a9705 100644 --- a/examples/featureflags.go +++ b/examples/featureflags.go @@ -8,11 +8,14 @@ import ( ) func TestIsFeatureEnabled() { - client, _ := posthog.NewWithConfig("phc_X8B6bhR1QgQKP1WdpFLN82LxLxgZ7WPXDgJyRyvIpib", posthog.Config{ - Interval: 30 * time.Second, - BatchSize: 100, - Verbose: true, - PersonalApiKey: "phx_vXZ7AOnFjDrCxfWLyo9V6P0SWLLfXT2d5euy3U0nRGk", + client, _ := posthog.NewWithConfig("phc_36WfBWNJEQcYotMZ7Ui7EWzqKLbIo2LWJFG5fIg1EER", posthog.Config{ + Interval: 30 * time.Second, + BatchSize: 100, + Verbose: true, + PersonalApiKey: "phx_n79cT52OfsxAWDhZs9j3w67aRoBCZ7l5ksRRKmAi5nr", + Endpoint: "http://localhost:8000", + DefaultFeatureFlagsPollingInterval: 5 * time.Second, + FeatureFlagRequestTimeout: 3 * time.Second, }) defer client.Close() @@ -22,9 +25,10 @@ func TestIsFeatureEnabled() { DistinctId: "hello", }) + fmt.Println("boolResult:", boolResult) + if boolErr != nil || boolResult == nil { fmt.Println("error:", boolErr) - return } // Simple flag @@ -32,9 +36,10 @@ func TestIsFeatureEnabled() { Key: "simple-test", DistinctId: "hello", }) + + fmt.Println("simpleResult:", simpleResult) if simpleErr != nil || simpleResult == false { fmt.Println("error:", simpleErr) - return } // Multivariate flag @@ -42,9 +47,9 @@ func TestIsFeatureEnabled() { Key: "multivariate-test", DistinctId: "hello", }) + fmt.Println("variantResult:", variantResult) if variantErr != nil || variantResult != "variant-value" { fmt.Println("error:", variantErr) - return } // Multivariate + simple flag @@ -52,8 +57,8 @@ func TestIsFeatureEnabled() { Key: "multivariate-simple-test", DistinctId: "hello", }) + fmt.Println("variantResult:", variantResult) if variantErr != nil || variantResult == true { fmt.Println("error:", variantErr) - return } } diff --git a/feature_flags_test.go b/feature_flags_test.go index 184de5d..c2aab19 100644 --- a/feature_flags_test.go +++ b/feature_flags_test.go @@ -1,12 +1,15 @@ package posthog import ( + "bytes" "encoding/json" "fmt" + "log" "net/http" "net/http/httptest" "reflect" "strings" + "time" "testing" ) @@ -613,9 +616,13 @@ func TestFeatureFlagNullComeIntoPlayOnlyWhenDecideErrorsOut(t *testing.T) { defer server.Close() + // TODO: Make this nicer, right now if all local evaluation requests fail, we block + // on waiting for atleast one request to happen before returning flags, + // which can be suboptimal client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{ - PersonalApiKey: "some very secret key", - Endpoint: server.URL, + PersonalApiKey: "some very secret key", + Endpoint: server.URL, + DefaultFeatureFlagsPollingInterval: 5 * time.Second, }) defer client.Close() @@ -3232,3 +3239,148 @@ func TestComplexCohortsWithNegationLocally(t *testing.T) { t.Error("Should match") } } + +func TestFlagWithTimeoutExceeded(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/decide") { + time.Sleep(1 * time.Second) + w.Write([]byte(fixture("test-decide-v2.json"))) + } else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") { + w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json"))) + } else if strings.HasPrefix(r.URL.Path, "/batch/") { + // Ignore batch requests + } else { + t.Error("Unknown request made by library") + } + })) + defer server.Close() + + client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{ + PersonalApiKey: "some very secret key", + Endpoint: server.URL, + FeatureFlagRequestTimeout: 10 * time.Millisecond, + }) + defer client.Close() + + isMatch, err := client.IsFeatureEnabled( + FeatureFlagPayload{ + Key: "enabled-flag", + DistinctId: "-", + }, + ) + + if err == nil { + t.Error("Expected error") + } + if !strings.Contains(err.Error(), "context deadline exceeded") { + t.Error("Expected context deadline exceeded error") + } + if isMatch != nil { + t.Error("Flag shouldn't match") + } + + // get all flags with no local evaluation possible + variants, err := client.GetAllFlags( + FeatureFlagPayloadNoKey{ + DistinctId: "-", + Groups: Groups{"company": "posthog"}, + }, + ) + + if err == nil { + t.Error("Expected error") + } + if !strings.Contains(err.Error(), "context deadline exceeded") { + t.Error("Expected context deadline exceeded error") + } + + if variants == nil || len(variants) != 0 { + t.Error("Flag shouldn't match") + } + + // get all flags with partial local evaluation possible + variants, err = client.GetAllFlags( + FeatureFlagPayloadNoKey{ + DistinctId: "-", + Groups: Groups{"company": "posthog"}, + PersonProperties: NewProperties().Set("region", "USA"), + }, + ) + + if err == nil { + t.Error("Expected error") + } + if !strings.Contains(err.Error(), "context deadline exceeded") { + t.Error("Expected context deadline exceeded error") + } + + if variants == nil || len(variants) != 1 || variants["simple-flag"] != true { + t.Error("should return locally evaluated flag") + } + + // get all flags with full local evaluation possible + variants, err = client.GetAllFlags( + FeatureFlagPayloadNoKey{ + DistinctId: "-", + Groups: Groups{"company": "posthog"}, + PersonProperties: NewProperties().Set("region", "USA"), + GroupProperties: map[string]Properties{"company": NewProperties().Set("name", "Project Name 1")}, + }, + ) + + if err != nil { + t.Error("Unexpected error") + } + fmt.Println(variants) + + if variants == nil || len(variants) != 2 || variants["simple-flag"] != true || variants["group-flag"] != true { + t.Error("should return locally evaluated flag") + } +} + +func TestFlagDefinitionsWithTimeoutExceeded(t *testing.T) { + + // create buffer to write logs to + var buf bytes.Buffer + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/decide") { + w.Write([]byte(fixture("test-decide-v2.json"))) + } else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") { + time.Sleep(11 * time.Second) + w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json"))) + } else if strings.HasPrefix(r.URL.Path, "/batch/") { + // Ignore batch requests + } else { + t.Error("Unknown request made by library") + } + })) + defer server.Close() + + client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{ + PersonalApiKey: "some very secret key", + Endpoint: server.URL, + FeatureFlagRequestTimeout: 10 * time.Millisecond, + Logger: StdLogger(log.New(&buf, "posthog-test", log.LstdFlags)), + }) + defer client.Close() + + _, err := client.IsFeatureEnabled( + FeatureFlagPayload{ + Key: "enabled-flag", + DistinctId: "-", + }, + ) + if err != nil { + t.Error("Unexpected error") + } + + output := buf.String() + if !strings.Contains(output, "Unable to fetch feature flags") { + t.Error("Expected error fetching flags") + } + + if !strings.Contains(output, "context deadline exceeded") { + t.Error("Expected timeout error fetching flags") + } +} diff --git a/featureflags.go b/featureflags.go index cc31f58..1a794bc 100644 --- a/featureflags.go +++ b/featureflags.go @@ -2,6 +2,7 @@ package posthog import ( "bytes" + "context" "crypto/sha1" "encoding/json" "errors" @@ -34,6 +35,7 @@ type FeatureFlagsPoller struct { mutex sync.RWMutex fetchedFlagsSuccessfullyOnce bool nextPollTick func() time.Duration + flagTimeout time.Duration } type FeatureFlag struct { @@ -121,6 +123,7 @@ func newFeatureFlagsPoller( httpClient http.Client, pollingInterval time.Duration, nextPollTick func() time.Duration, + flagTimeout time.Duration, ) *FeatureFlagsPoller { if nextPollTick == nil { @@ -139,6 +142,7 @@ func newFeatureFlagsPoller( mutex: sync.RWMutex{}, fetchedFlagsSuccessfullyOnce: false, nextPollTick: nextPollTick, + flagTimeout: flagTimeout, } go poller.run() @@ -169,7 +173,8 @@ func (poller *FeatureFlagsPoller) run() { func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { personalApiKey := poller.personalApiKey headers := [][2]string{{"Authorization", "Bearer " + personalApiKey + ""}} - res, err := poller.localEvaluationFlags(headers) + res, cancel, err := poller.localEvaluationFlags(headers) + defer cancel() if err != nil || res.StatusCode != http.StatusOK { poller.loaded <- false poller.Errorf("Unable to fetch feature flags", err) @@ -193,9 +198,7 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { poller.loaded <- true } newFlags := []FeatureFlag{} - for _, flag := range featureFlagsResponse.Flags { - newFlags = append(newFlags, flag) - } + newFlags = append(newFlags, featureFlagsResponse.Flags...) poller.mutex.Lock() poller.featureFlags = newFlags poller.cohorts = featureFlagsResponse.Cohorts @@ -242,7 +245,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload) result, err = poller.getFeatureFlagVariant(featureFlag, flagConfig.Key, flagConfig.DistinctId, flagConfig.Groups, flagConfig.PersonProperties, flagConfig.GroupProperties) if err != nil { - return nil, nil + return nil, err } } @@ -746,7 +749,7 @@ func interfaceToFloat(val interface{}) (float64, error) { case uint64: i = float64(t) default: - errMessage := "Argument not orderable" + errMessage := "argument not orderable" return 0.0, errors.New(errMessage) } @@ -815,18 +818,18 @@ func (poller *FeatureFlagsPoller) GetFeatureFlags() []FeatureFlag { return poller.featureFlags } -func (poller *FeatureFlagsPoller) decide(requestData []byte, headers [][2]string) (*http.Response, error) { - localEvaluationEndpoint := "decide/?v=2" +func (poller *FeatureFlagsPoller) decide(requestData []byte, headers [][2]string) (*http.Response, context.CancelFunc, error) { + decideEndpoint := "decide/?v=2" - url, err := url.Parse(poller.Endpoint + "/" + localEvaluationEndpoint + "") + url, err := url.Parse(poller.Endpoint + "/" + decideEndpoint + "") if err != nil { poller.Errorf("creating url - %s", err) } - return poller.request("POST", url, requestData, headers) + return poller.request("POST", url, requestData, headers, poller.flagTimeout) } -func (poller *FeatureFlagsPoller) localEvaluationFlags(headers [][2]string) (*http.Response, error) { +func (poller *FeatureFlagsPoller) localEvaluationFlags(headers [][2]string) (*http.Response, context.CancelFunc, error) { localEvaluationEndpoint := "api/feature_flag/local_evaluation" url, err := url.Parse(poller.Endpoint + "/" + localEvaluationEndpoint + "") @@ -838,11 +841,13 @@ func (poller *FeatureFlagsPoller) localEvaluationFlags(headers [][2]string) (*ht searchParams.Add("send_cohorts", "true") url.RawQuery = searchParams.Encode() - return poller.request("GET", url, []byte{}, headers) + return poller.request("GET", url, []byte{}, headers, time.Duration(10)*time.Second) } -func (poller *FeatureFlagsPoller) request(method string, url *url.URL, requestData []byte, headers [][2]string) (*http.Response, error) { - req, err := http.NewRequest(method, url.String(), bytes.NewReader(requestData)) +func (poller *FeatureFlagsPoller) request(method string, url *url.URL, requestData []byte, headers [][2]string, timeout time.Duration) (*http.Response, context.CancelFunc, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + + req, err := http.NewRequestWithContext(ctx, method, url.String(), bytes.NewReader(requestData)) if err != nil { poller.Errorf("creating request - %s", err) } @@ -862,7 +867,7 @@ func (poller *FeatureFlagsPoller) request(method string, url *url.URL, requestDa poller.Errorf("sending request - %s", err) } - return res, err + return res, cancel, err } func (poller *FeatureFlagsPoller) ForceReload() { @@ -888,9 +893,13 @@ func (poller *FeatureFlagsPoller) getFeatureFlagVariants(distinctId string, grou poller.Errorf(errorMessage) return nil, errors.New(errorMessage) } - res, err := poller.decide(requestDataBytes, headers) + res, cancel, err := poller.decide(requestDataBytes, headers) + defer cancel() if err != nil || res.StatusCode != http.StatusOK { errorMessage = "Error calling /decide/" + if err != nil { + errorMessage += " - " + err.Error() + } poller.Errorf(errorMessage) return nil, errors.New(errorMessage) } diff --git a/fixtures/feature_flag/test-flag-group-properties.json b/fixtures/feature_flag/test-flag-group-properties.json index 4b42d65..a956466 100644 --- a/fixtures/feature_flag/test-flag-group-properties.json +++ b/fixtures/feature_flag/test-flag-group-properties.json @@ -1,5 +1,5 @@ { - "count": 1, + "count": 2, "next": null, "previous": null, "flags": [ @@ -28,6 +28,30 @@ "active": true, "is_simple_flag": false, "rollout_percentage": null + }, + { + "id": 719, + "name": "", + "key": "simple-flag", + "filters": { + "groups": [ + { + "properties": [ + { + "key": "region", + "operator": "exact", + "value": ["USA"], + "type": "person" + } + ], + "rollout_percentage": 100 + } + ] + }, + "deleted": false, + "active": true, + "is_simple_flag": true, + "rollout_percentage": null } ], "group_type_mapping" : {"0": "company", "1": "project"} diff --git a/posthog.go b/posthog.go index f38aed7..16a8c02 100644 --- a/posthog.go +++ b/posthog.go @@ -119,6 +119,7 @@ func NewWithConfig(apiKey string, config Config) (cli Client, err error) { c.http, c.DefaultFeatureFlagsPollingInterval, c.NextFeatureFlagsPollingTick, + c.FeatureFlagRequestTimeout, ) }