do not throw error on cohort download failure, log error on evaluate

pkg/experiment/local/client.go

@@ -116,13 +116,14 @@ func (c *Client) Evaluate(user *experiment.User, flagKeys []string) (map[string]
 
 func (c *Client) EvaluateV2(user *experiment.User, flagKeys []string) (map[string]experiment.Variant, error) {
 	flagConfigs := c.flagConfigStorage.getFlagConfigs()
-	enrichedUser, err := c.enrichUser(user, flagConfigs)
+	sortedFlags, err := topologicalSort(flagConfigs, flagKeys)
+	c.requiredCohortsInStorage(sortedFlags)
+	enrichedUser, err := c.enrichUserWithCohorts(user, flagConfigs)
 	if err != nil {
 		return nil, err
 	}
 	userContext := evaluation.UserToContext(enrichedUser)
 	c.flagsMutex.RLock()
-	sortedFlags, err := topologicalSort(flagConfigs, flagKeys)
 	c.flagsMutex.RUnlock()
 	if err != nil {
 		return nil, err
@@ -338,7 +339,29 @@ func coerceString(value interface{}) string {
 	return fmt.Sprintf("%v", value)
 }
 
-func (c *Client) enrichUser(user *experiment.User, flagConfigs map[string]*evaluation.Flag) (*experiment.User, error) {
+func (c *Client) requiredCohortsInStorage(flagConfigs []*evaluation.Flag) {
+	storedCohortIDs := c.cohortStorage.getCohortIds()
+	for _, flag := range flagConfigs {
+		flagCohortIDs := getAllCohortIDsFromFlag(flag)
+		missingCohorts := difference(flagCohortIDs, storedCohortIDs)
+
+		if len(missingCohorts) > 0 {
+			if c.config.CohortSyncConfig != nil {
+				c.log.Debug(
+					"Evaluating flag %s dependent on cohorts %v without %v in storage",
+					flag.Key, flagCohortIDs, missingCohorts,
+				)
+			} else {
+				c.log.Debug(
+					"Evaluating flag %s dependent on cohorts %v without cohort syncing configured",
+					flag.Key, flagCohortIDs,
+				)
+			}
+		}
+	}
+}
+
+func (c *Client) enrichUserWithCohorts(user *experiment.User, flagConfigs map[string]*evaluation.Flag) (*experiment.User, error) {
 	flagConfigSlice := make([]*evaluation.Flag, 0, len(flagConfigs))
 
 	for _, value := range flagConfigs {

pkg/experiment/local/cohort_download_api.go

@@ -88,7 +88,7 @@ func (api *directCohortDownloadApi) getCohort(cohortID string, cohort *Cohort) (
 			api.log.Debug("getCohortMembers(%s): Cohort not modified", cohortID)
 			return nil, nil
 		} else if response.StatusCode == http.StatusRequestEntityTooLarge {
-			return nil, &CohortTooLargeException{Message: "Cohort exceeds max cohort size"}
+			return nil, &CohortTooLargeException{Message: "Cohort exceeds max cohort size of " + strconv.Itoa(api.MaxCohortSize)}
 		} else {
 			return nil, &HTTPErrorResponseException{StatusCode: response.StatusCode, Message: "Unexpected response code"}
 		}

pkg/experiment/local/deployment_runner.go

@@ -2,7 +2,6 @@ package local
 
 import (
 	"fmt"
-	"strings"
 	"sync"
 
 	"github.com/amplitude/experiment-go-server/internal/evaluation"
@@ -119,31 +118,20 @@ func (dr *deploymentRunner) updateFlagConfigs() error {
 	// Get updated set of cohort ids
 	updatedCohortIDs := dr.cohortStorage.getCohortIds()
 	// Iterate through new flag configs and check if their required cohorts exist
-	failedFlagCount := 0
 	for _, flagConfig := range flagConfigs {
 		cohortIDs := getAllCohortIDsFromFlag(flagConfig)
-		if len(cohortIDs) == 0 || dr.cohortLoader == nil {
-			dr.flagConfigStorage.putFlagConfig(flagConfig)
-			dr.log.Debug("Putting non-cohort flag %s", flagConfig.Key)
-		} else if subset(cohortIDs, updatedCohortIDs) {
-			dr.flagConfigStorage.putFlagConfig(flagConfig)
-			dr.log.Debug("Putting flag %s", flagConfig.Key)
-		} else {
-			dr.log.Error("Flag %s not updated because not all required cohorts could be loaded", flagConfig.Key)
-			failedFlagCount++
+		missingCohorts := difference(cohortIDs, updatedCohortIDs)
+
+		dr.flagConfigStorage.putFlagConfig(flagConfig)
+		dr.log.Debug("Putting flag %s", flagConfig.Key)
+		if len(missingCohorts) != 0 {
+			dr.log.Error("Flag %s - failed to load cohorts: %v", flagConfig.Key, missingCohorts)
 		}
 	}
 
 	// Delete unused cohorts
 	dr.deleteUnusedCohorts()
-	dr.log.Debug("Refreshed %d flag configs.", len(flagConfigs)-failedFlagCount)
-
-	// If there are any download errors, raise an aggregated exception
-	if len(cohortDownloadErrors) > 0 {
-		errorCount := len(cohortDownloadErrors)
-		errorMessages := strings.Join(cohortDownloadErrors, "\n")
-		return fmt.Errorf("%d cohort(s) failed to download:\n%s", errorCount, errorMessages)
-	}
+	dr.log.Debug("Refreshed %d flag configs.", len(flagConfigs))
 
 	return nil
 }
@@ -183,12 +171,3 @@ func difference(set1, set2 map[string]struct{}) map[string]struct{} {
 	}
 	return diff
 }
-
-func subset(subset, set map[string]struct{}) bool {
-	for k := range subset {
-		if _, exists := set[k]; !exists {
-			return false
-		}
-	}
-	return true
-}

pkg/experiment/local/deployment_runner_test.go

@@ -36,7 +36,7 @@ func TestStartThrowsIfFirstFlagConfigLoadFails(t *testing.T) {
 	}
 }
 
-func TestStartThrowsIfFirstCohortLoadFails(t *testing.T) {
+func TestStartSucceedsEvenIfFirstCohortLoadFails(t *testing.T) {
 	flagAPI := &mockFlagConfigApi{getFlagConfigsFunc: func() (map[string]*evaluation.Flag, error) {
 		return map[string]*evaluation.Flag{"flag": createTestFlag()}, nil
 	}}
@@ -57,8 +57,8 @@ func TestStartThrowsIfFirstCohortLoadFails(t *testing.T) {
 
 	err := runner.start()
 
-	if err == nil {
-		t.Error("Expected error but got nil")
+	if err != nil {
+		t.Errorf("Expected no error but got %v", err)
 	}
 }