Add more error handling to feature exper and composite feature service

Author: Mat001

pkg/decision/composite_feature_service.go

@@ -18,6 +18,8 @@
 package decision
 
 import (
+	"strings"
+
 	"github.com/optimizely/go-sdk/v2/pkg/decide"
 	"github.com/optimizely/go-sdk/v2/pkg/entities"
 	"github.com/optimizely/go-sdk/v2/pkg/logging"
@@ -51,6 +53,19 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont
 		reasons.Append(decisionReasons)
 		if err != nil {
 			f.logger.Debug(err.Error())
+			// Check if this is a CMAB error - if so, stop the loop and return empty decision
+			if strings.Contains(err.Error(), "Failed to fetch CMAB data") {
+				return FeatureDecision{}, reasons, nil // Return empty decision for CMAB errors
+			}
+		}
+
+		// Also check for CMAB errors in decision reasons (when err is nil)
+		if decisionReasons != nil {
+			for _, reason := range decisionReasons.ToReport() {
+				if strings.Contains(reason, "Failed to fetch CMAB data") {
+					return FeatureDecision{}, reasons, nil
+				}
+			}
 		}
 
 		if featureDecision.Variation != nil && err == nil {

pkg/decision/composite_feature_service_test.go

@@ -109,15 +109,16 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionFallthrough() {
 }
 
 func (s *CompositeFeatureServiceTestSuite) TestGetDecisionReturnsError() {
-	// test that we move onto the next decision service if an inner service returns an error
+	// test that we move onto the next decision service if an inner service returns a non-CMAB error
 	testUserContext := entities.UserContext{
 		ID: "test_user_1",
 	}
 
 	shouldBeIgnoredDecision := FeatureDecision{
 		Variation: &testExp1113Var2223,
 	}
-	s.mockFeatureService.On("GetDecision", s.testFeatureDecisionContext, testUserContext, s.options).Return(shouldBeIgnoredDecision, s.reasons, errors.New("Error making decision"))
+	// Use a non-CMAB error to ensure fallthrough still works for other errors
+	s.mockFeatureService.On("GetDecision", s.testFeatureDecisionContext, testUserContext, s.options).Return(shouldBeIgnoredDecision, s.reasons, errors.New("Generic experiment error"))
 
 	expectedDecision := FeatureDecision{
 		Variation: &testExp1113Var2224,
@@ -165,6 +166,38 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionReturnsLastDecisionWit
 	s.mockFeatureService2.AssertExpectations(s.T())
 }
 
+func (s *CompositeFeatureServiceTestSuite) TestGetDecisionWithCmabError() {
+	// Test that CMAB errors are terminal and don't fall through to rollout service
+	testUserContext := entities.UserContext{
+		ID: "test_user_1",
+	}
+
+	// Mock the first service (FeatureExperimentService) to return a CMAB error
+	cmabError := errors.New("Failed to fetch CMAB data for experiment exp_1.")
+	emptyDecision := FeatureDecision{}
+	s.mockFeatureService.On("GetDecision", s.testFeatureDecisionContext, testUserContext, s.options).Return(emptyDecision, s.reasons, cmabError)
+
+	// The second service (RolloutService) should NOT be called for CMAB errors
+
+	compositeFeatureService := &CompositeFeatureService{
+		featureServices: []FeatureService{
+			s.mockFeatureService,
+			s.mockFeatureService2,
+		},
+		logger: logging.GetLogger("sdkKey", "CompositeFeatureService"),
+	}
+
+	decision, _, err := compositeFeatureService.GetDecision(s.testFeatureDecisionContext, testUserContext, s.options)
+
+	// CMAB errors should result in empty decision with no error
+	s.Equal(FeatureDecision{}, decision)
+	s.NoError(err, "CMAB errors should not propagate as Go errors")
+
+	s.mockFeatureService.AssertExpectations(s.T())
+	// Verify that the rollout service was NOT called
+	s.mockFeatureService2.AssertNotCalled(s.T(), "GetDecision")
+}
+
 func (s *CompositeFeatureServiceTestSuite) TestNewCompositeFeatureService() {
 	// Assert that the service is instantiated with the correct child services in the right order
 	compositeExperimentService := NewCompositeExperimentService("")

pkg/decision/feature_experiment_service.go

@@ -79,7 +79,7 @@ func (f FeatureExperimentService) GetDecision(decisionContext FeatureDecisionCon
 		// Handle CMAB experiment errors - they should terminate the decision process
 		if err != nil && experiment.Cmab != nil {
 			// For CMAB experiments, errors should prevent fallback to other experiments
-			// Return empty FeatureDecision (enabled: false, variation_key: null, rule_key: null)
+			// The error is already in reasons from decisionReasons, so return nil error
 			return FeatureDecision{}, reasons, nil
 		}
 

pkg/decision/feature_experiment_service_test.go

@@ -17,6 +17,7 @@
 package decision
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/optimizely/go-sdk/v2/pkg/decide"
@@ -230,6 +231,69 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecisionWithCmabUUID() {
 	s.mockExperimentService.AssertExpectations(s.T())
 }
 
+func (s *FeatureExperimentServiceTestSuite) TestGetDecisionWithCmabError() {
+	testUserContext := entities.UserContext{
+		ID: "test_user_1",
+	}
+
+	// Create a NEW CMAB experiment (don't modify existing testExp1113)
+	cmabExperiment := entities.Experiment{
+		ID:  "cmab_experiment_id",
+		Key: "cmab_experiment_key",
+		Cmab: &entities.Cmab{
+			AttributeIds:      []string{"attr1", "attr2"},
+			TrafficAllocation: 5000, // 50%
+		},
+		Variations: testExp1113.Variations, // Reuse variations for simplicity
+	}
+
+	// Setup experiment decision context for CMAB experiment
+	testExperimentDecisionContext := ExperimentDecisionContext{
+		Experiment:    &cmabExperiment,
+		ProjectConfig: s.mockConfig,
+	}
+
+	// Mock the experiment service to return a CMAB error
+	cmabError := errors.New("Failed to fetch CMAB data for experiment cmab_experiment_key.")
+	s.mockExperimentService.On("GetDecision", testExperimentDecisionContext, testUserContext, s.options).
+		Return(ExperimentDecision{}, s.reasons, cmabError)
+
+	// Create a test feature that uses our CMAB experiment
+	testFeatureWithCmab := entities.Feature{
+		ID:  "test_feature_cmab",
+		Key: "test_feature_cmab_key",
+		FeatureExperiments: []entities.Experiment{
+			cmabExperiment, // Only our CMAB experiment
+		},
+	}
+
+	// Create feature decision context with our CMAB feature
+	testFeatureDecisionContextWithCmab := FeatureDecisionContext{
+		Feature:               &testFeatureWithCmab,
+		ProjectConfig:         s.mockConfig,
+		Variable:              testVariable,
+		ForcedDecisionService: NewForcedDecisionService("test_user"),
+	}
+
+	// Create service under test
+	featureExperimentService := &FeatureExperimentService{
+		compositeExperimentService: s.mockExperimentService,
+		logger:                     logging.GetLogger("sdkKey", "FeatureExperimentService"),
+	}
+
+	// Call GetDecision
+	actualFeatureDecision, actualReasons, err := featureExperimentService.GetDecision(testFeatureDecisionContextWithCmab, testUserContext, s.options)
+
+	// Verify that CMAB error results in empty feature decision (not error)
+	s.NoError(err, "CMAB errors should not propagate as Go errors")
+	s.Equal(FeatureDecision{}, actualFeatureDecision, "Should return empty FeatureDecision when CMAB fails")
+
+	// Verify that reasons include the CMAB error (should be in actualReasons from mock)
+	s.NotNil(actualReasons, "Decision reasons should not be nil")
+
+	s.mockExperimentService.AssertExpectations(s.T())
+}
+
 func TestFeatureExperimentServiceTestSuite(t *testing.T) {
 	suite.Run(t, new(FeatureExperimentServiceTestSuite))
 }