fix(flags): Add configurable flag timeouts (#37)

.github/workflows/unit-tests.yml

@@ -20,4 +20,4 @@ jobs:
         run: go build .
 
       - name: Test
-        run: go test -v .
+        run: go test -timeout 30s -v .

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
 	}

examples/capture.go

@@ -2,8 +2,9 @@ package main
 
 import (
 	"fmt"
-	"github.com/posthog/posthog-go"
 	"time"
+
+	"github.com/posthog/posthog-go"
 )
 
 func TestCapture() {

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,38 +25,40 @@ func TestIsFeatureEnabled() {
 			DistinctId: "hello",
 		})
 
+	fmt.Println("boolResult:", boolResult)
+
 	if boolErr != nil || boolResult == nil {
 		fmt.Println("error:", boolErr)
-		return
 	}
 
 	// Simple flag
 	simpleResult, simpleErr := client.GetFeatureFlag(posthog.FeatureFlagPayload{
 		Key:        "simple-test",
 		DistinctId: "hello",
 	})
+
+	fmt.Println("simpleResult:", simpleResult)
 	if simpleErr != nil || simpleResult == false {
 		fmt.Println("error:", simpleErr)
-		return
 	}
 
 	// Multivariate flag
 	variantResult, variantErr := client.GetFeatureFlag(posthog.FeatureFlagPayload{
 		Key:        "multivariate-test",
 		DistinctId: "hello",
 	})
+	fmt.Println("variantResult:", variantResult)
 	if variantErr != nil || variantResult != "variant-value" {
 		fmt.Println("error:", variantErr)
-		return
 	}
 
 	// Multivariate + simple flag
 	variantResult, variantErr = client.GetFeatureFlag(posthog.FeatureFlagPayload{
 		Key:        "multivariate-simple-test",
 		DistinctId: "hello",
 	})
+	fmt.Println("variantResult:", variantResult)
 	if variantErr != nil || variantResult == true {
 		fmt.Println("error:", variantErr)
-		return
 	}
 }

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")
+	}
+}