fix: nil pointer if Reason field is nil (#262)

pkg/eventpersister/persister/persister.go

@@ -41,6 +41,7 @@ var (
 	ErrNoAutoOpsRules        = errors.New("eventpersister: no auto ops rules")
 	ErrNoExperiments         = errors.New("eventpersister: no experiments")
 	ErrNothingToLink         = errors.New("eventpersister: nothing to link")
+	ErrReasonNil             = errors.New("eventpersister: reason is nil")
 )
 
 const (
@@ -285,16 +286,22 @@ func (p *Persister) extractEvents(messages map[string]*puller.Message) environme
 	return envEvents
 }
 
-func getVariationID(reason featureproto.Reason_Type, vID string) string {
-	if reason == featureproto.Reason_CLIENT {
-		return defaultVariationID
+func getVariationID(reason *featureproto.Reason, vID string) (string, error) {
+	if reason == nil {
+		return "", ErrReasonNil
 	}
-	return vID
+	if reason.Type == featureproto.Reason_CLIENT {
+		return defaultVariationID, nil
+	}
+	return vID, nil
 }
 
 func (p *Persister) upsertEvaluationCount(event proto.Message, environmentNamespace string) error {
 	if e, ok := event.(*eventproto.EvaluationEvent); ok {
-		vID := getVariationID(e.Reason.Type, e.VariationId)
+		vID, err := getVariationID(e.Reason, e.VariationId)
+		if err != nil {
+			return err
+		}
 		// To avoid duplication when the request fails, we increment the event count in the end
 		// because the user count is an unique count, and there is no problem adding the same event more than once
 		uck := p.newEvaluationCountkey(userCountKey, e.FeatureId, vID, environmentNamespace, e.Timestamp)

pkg/eventpersister/persister/persister_test.go

@@ -131,44 +131,62 @@ func TestGetVariationID(t *testing.T) {
 	patterns := []struct {
 		desc        string
 		variationID string
-		reason      featureproto.Reason_Type
+		reason      *featureproto.Reason
 		expected    string
+		expectedErr error
 	}{
 		{
 			desc:        "get given variation id if off variation",
 			variationID: "vID1",
-			reason:      featureproto.Reason_OFF_VARIATION,
-			expected:    "vID1",
+			reason: &featureproto.Reason{
+				Type: featureproto.Reason_OFF_VARIATION,
+			},
+			expected: "vID1",
 		},
 		{
 			desc:        "get given variation id if target",
 			variationID: "vID1",
-			reason:      featureproto.Reason_TARGET,
-			expected:    "vID1",
+			reason: &featureproto.Reason{
+				Type: featureproto.Reason_TARGET,
+			},
+			expected: "vID1",
 		},
 		{
 			desc:        "get given variation id if rule",
 			variationID: "vID1",
-			reason:      featureproto.Reason_RULE,
-			expected:    "vID1",
+			reason: &featureproto.Reason{
+				Type: featureproto.Reason_RULE,
+			},
+			expected: "vID1",
 		},
 		{
 			desc:        "get given variation id if prerequisite",
 			variationID: "vID1",
-			reason:      featureproto.Reason_PREREQUISITE,
-			expected:    "vID1",
+			reason: &featureproto.Reason{
+				Type: featureproto.Reason_PREREQUISITE,
+			},
+			expected: "vID1",
+		},
+		{
+			desc:        "get given variation id if reason is nil",
+			variationID: "vID1",
+			reason:      nil,
+			expectedErr: ErrReasonNil,
 		},
 		{
 			desc:        "get default variation id if client",
 			variationID: "vID1",
-			reason:      featureproto.Reason_CLIENT,
-			expected:    defaultVariationID,
+			reason: &featureproto.Reason{
+				Type: featureproto.Reason_CLIENT,
+			},
+			expected: defaultVariationID,
 		},
 	}
 	for _, p := range patterns {
 		t.Run(p.desc, func(t *testing.T) {
-			actual := getVariationID(p.reason, p.variationID)
+			actual, err := getVariationID(p.reason, p.variationID)
 			assert.Equal(t, p.expected, actual)
+			assert.Equal(t, p.expectedErr, err)
 		})
 	}
 }