Skip to content

Commit

Permalink
fix: Handle errors in flagState conversion (#2455)
Browse files Browse the repository at this point in the history
* Handle errors in flagState conversion

* Changing the error details

* Add test for coverage

* Add test for value
  • Loading branch information
thomaspoignant authored Oct 4, 2024
1 parent b5144e3 commit a680901
Show file tree
Hide file tree
Showing 5 changed files with 228 additions and 2 deletions.
12 changes: 12 additions & 0 deletions gofferror/empty_bucketing_key.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package gofferror

import "fmt"

type EmptyBucketingKeyError struct {
Message string
}

// Implement the Error() method for the custom error type
func (e *EmptyBucketingKeyError) Error() string {
return fmt.Sprintf("Error: %s", e.Message)
}
11 changes: 9 additions & 2 deletions internal/flag/internal_flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/thomaspoignant/go-feature-flag/ffcontext"
"github.com/thomaspoignant/go-feature-flag/gofferror"
"github.com/thomaspoignant/go-feature-flag/internal/internalerror"
"github.com/thomaspoignant/go-feature-flag/internal/utils"
)
Expand Down Expand Up @@ -67,7 +68,6 @@ func (f *InternalFlag) Value(
) (interface{}, ResolutionDetails) {
evaluationDate := DateFromContextOrDefault(evaluationCtx, time.Now())
flag, err := f.applyScheduledRolloutSteps(evaluationDate)

if err != nil {
return flagContext.DefaultSdkValue, ResolutionDetails{
Variant: VariationSDKDefault,
Expand All @@ -81,7 +81,6 @@ func (f *InternalFlag) Value(
}

key, keyError := flag.GetBucketingKeyValue(evaluationCtx)

if keyError != nil {
return flagContext.DefaultSdkValue, ResolutionDetails{
Variant: VariationSDKDefault,
Expand Down Expand Up @@ -398,11 +397,19 @@ func (f *InternalFlag) GetBucketingKeyValue(ctx ffcontext.Context) (string, erro
value := ctx.GetCustom()[key]
switch v := value.(type) {
case string:
if v == "" {
return "", &gofferror.EmptyBucketingKeyError{Message: "Empty bucketing key"}
}
return v, nil
default:
return "", fmt.Errorf("invalid bucketing key")
}
}

if ctx.GetKey() == "" {
return "", &gofferror.EmptyBucketingKeyError{Message: "Empty targeting key"}
}

return ctx.GetKey(), nil
}

Expand Down
71 changes: 71 additions & 0 deletions internal/flag/internal_flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,77 @@ func TestInternalFlag_Value(t *testing.T) {
ErrorCode: flag.ErrorCodeGeneral,
},
},
{
name: "Empty targeting key",
flag: flag.InternalFlag{
Variations: &map[string]*interface{}{
"variation_A": testconvert.Interface(true),
"variation_B": testconvert.Interface(false),
},
DefaultRule: &flag.Rule{
VariationResult: testconvert.String("variation_A"),
},
Metadata: &map[string]interface{}{
"description": "this is a flag",
"issue-link": "https://issue.link/GOFF-1",
},
},
args: args{
flagName: "my-flag",
user: ffcontext.NewEvaluationContext(""),
flagContext: flag.Context{
DefaultSdkValue: false,
},
},
want: false,
want1: flag.ResolutionDetails{
Variant: "SdkDefault",
Reason: flag.ReasonError,
ErrorCode: flag.ErrorCodeTargetingKeyMissing,
ErrorMessage: "Error: Empty targeting key",
Cacheable: false,
Metadata: map[string]interface{}{
"description": "this is a flag",
"issue-link": "https://issue.link/GOFF-1",
},
},
},
{
name: "Empty bucketing key",
flag: flag.InternalFlag{
Variations: &map[string]*interface{}{
"variation_A": testconvert.Interface(true),
"variation_B": testconvert.Interface(false),
},
DefaultRule: &flag.Rule{
VariationResult: testconvert.String("variation_A"),
},
Metadata: &map[string]interface{}{
"description": "this is a flag",
"issue-link": "https://issue.link/GOFF-1",
},
BucketingKey: testconvert.String("teamId"),
},
args: args{
flagName: "my-flag",
user: ffcontext.NewEvaluationContextBuilder("toto").AddCustom("teamId", "").Build(),
flagContext: flag.Context{
DefaultSdkValue: false,
},
},
want: false,
want1: flag.ResolutionDetails{
Variant: "SdkDefault",
Reason: flag.ReasonError,
ErrorCode: flag.ErrorCodeTargetingKeyMissing,
ErrorMessage: "Error: Empty bucketing key",
Cacheable: false,
Metadata: map[string]interface{}{
"description": "this is a flag",
"issue-link": "https://issue.link/GOFF-1",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
12 changes: 12 additions & 0 deletions internal/flagstate/flag_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ func FromFlagEvaluation(key string, evaluationCtx ffcontext.Context,
}
}

if resolutionDetails.Reason == flag.ReasonError {
return FlagState{
Timestamp: time.Now().Unix(),
TrackEvents: currentFlag.IsTrackEvents(),
Failed: resolutionDetails.ErrorCode != "",
ErrorCode: resolutionDetails.ErrorCode,
ErrorDetails: resolutionDetails.ErrorMessage,
Reason: resolutionDetails.Reason,
Metadata: resolutionDetails.Metadata,
}
}

switch v := flagValue; v.(type) {
case int, float64, bool, string, []interface{}, map[string]interface{}:
return FlagState{
Expand Down
124 changes: 124 additions & 0 deletions internal/flagstate/flag_state_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package flagstate_test

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/thomaspoignant/go-feature-flag/ffcontext"
"github.com/thomaspoignant/go-feature-flag/internal/flag"
"github.com/thomaspoignant/go-feature-flag/internal/flagstate"
"github.com/thomaspoignant/go-feature-flag/testutils/testconvert"
)

func TestFromFlagEvaluation(t *testing.T) {
tests := []struct {
name string
key string
evaluationCtx ffcontext.Context
flagCtx flag.Context
currentFlag flag.Flag
expected flagstate.FlagState
}{
{
name: "Flag is disabled",
key: "test-key",
evaluationCtx: ffcontext.NewEvaluationContext("user-key"),
flagCtx: flag.Context{},
currentFlag: &flag.InternalFlag{
Disable: testconvert.Bool(true),
},
expected: flagstate.FlagState{
Timestamp: time.Now().Unix(),
TrackEvents: true,
Failed: false,
ErrorCode: "",
ErrorDetails: "",
Reason: flag.ReasonDisabled,
Metadata: nil,
},
},
{
name: "Flag evaluation error",
key: "test-key",
evaluationCtx: ffcontext.NewEvaluationContext(""),
flagCtx: flag.Context{},
currentFlag: &flag.InternalFlag{
Disable: testconvert.Bool(false),
Variations: &map[string]*interface{}{
"var1": testconvert.Interface(1),
"var2": testconvert.Interface(2),
},
DefaultRule: &flag.Rule{
VariationResult: testconvert.String("var1"),
},
},
expected: flagstate.FlagState{
Timestamp: time.Now().Unix(),
TrackEvents: true,
Failed: true,
ErrorCode: flag.ErrorCodeTargetingKeyMissing,
ErrorDetails: "Error: Empty targeting key",
Reason: flag.ReasonError,
Metadata: nil,
},
},
{
name: "Flag evaluation valid type",
key: "test-key",
evaluationCtx: ffcontext.NewEvaluationContext("my-key"),
flagCtx: flag.Context{},
currentFlag: &flag.InternalFlag{
Disable: testconvert.Bool(false),
Variations: &map[string]*interface{}{
"var1": testconvert.Interface(1),
"var2": testconvert.Interface(2),
},
DefaultRule: &flag.Rule{
VariationResult: testconvert.String("var1"),
},
},
expected: flagstate.FlagState{
Value: 1,
VariationType: "var1",
Timestamp: time.Now().Unix(),
TrackEvents: true,
Failed: false,
Reason: flag.ReasonStatic,
Metadata: nil,
},
},
{
name: "Flag evaluation invalid type",
key: "test-key",
evaluationCtx: ffcontext.NewEvaluationContext("my-key"),
flagCtx: flag.Context{},
currentFlag: &flag.InternalFlag{
Disable: testconvert.Bool(false),
Variations: &map[string]*interface{}{
"var1": testconvert.Interface(map[bool]*interface{}{true: testconvert.Interface(1)}),
"var2": testconvert.Interface(map[bool]*interface{}{true: testconvert.Interface(2)}),
},
DefaultRule: &flag.Rule{
VariationResult: testconvert.String("var1"),
},
},
expected: flagstate.FlagState{
VariationType: "SdkDefault",
Timestamp: time.Now().Unix(),
TrackEvents: true,
Failed: true,
Reason: flag.ReasonError,
ErrorCode: flag.ErrorCodeTypeMismatch,
Metadata: nil,
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := flagstate.FromFlagEvaluation(tt.key, tt.evaluationCtx, tt.flagCtx, tt.currentFlag)
assert.Equal(t, tt.expected, got)
})
}
}

0 comments on commit a680901

Please sign in to comment.