Commit 3131a68b authored by Doug Barrett's avatar Doug Barrett 🔴
Browse files

fix(v2/featureflag): skip caching error evaluation results 

Transient provider failures (e.g. Flipt timeout, connection refused)
return a ResolutionError in the BoolResolutionDetail/StringResolutionDetail.
Previously these error results were cached for the full TTL, meaning a
brief outage could poison the cache and prevent recovery until entries
expire.

Check ProviderResolutionDetail.Error() before writing to cache. Only
successful evaluations are now cached; error results always delegate to
the inner provider on the next call.

Ref: https://pkg.go.dev/github.com/open-feature/go-sdk/openfeature#ProviderResolutionDetail.Error
parent ac35084a

v2/featureflag/cache.go

+12 −6
Original line number Diff line number Diff line
@@ -82,17 +82,20 @@ func (p *cachingProvider) BooleanEvaluation( 
	defaultValue bool,

	evalCtx openfeature.FlattenedContext,

) openfeature.BoolResolutionDetail {

	key, err := buildCacheKey(flag, evalCtx)

	if err == nil {

	key, keyErr := buildCacheKey(flag, evalCtx)

	if keyErr == nil {

		if detail, ok := p.boolCache.Get(key); ok {

			detail.Reason = openfeature.CachedReason

			return detail

		}

	}



	result := p.inner.BooleanEvaluation(ctx, flag, defaultValue, evalCtx)

	if err == nil {



	if keyErr == nil && result.Error() == nil {

		p.boolCache.Add(key, result)

	}



	return result

}
@@ -104,17 +107,20 @@ func (p *cachingProvider) StringEvaluation( 
	defaultValue string,

	evalCtx openfeature.FlattenedContext,

) openfeature.StringResolutionDetail {

	key, err := buildCacheKey(flag, evalCtx)

	if err == nil {

	key, keyErr := buildCacheKey(flag, evalCtx)

	if keyErr == nil {

		if detail, ok := p.stringCache.Get(key); ok {

			detail.Reason = openfeature.CachedReason

			return detail

		}

	}



	result := p.inner.StringEvaluation(ctx, flag, defaultValue, evalCtx)

	if err == nil {



	if keyErr == nil && result.Error() == nil {

		p.stringCache.Add(key, result)

	}



	return result

}

v2/featureflag/cache_test.go

+197 −0
Original line number Diff line number Diff line
@@ -41,6 +41,97 @@ func (p *staticProvider) ObjectEvaluation(_ context.Context, _ string, d any, _ 
	return openfeature.InterfaceResolutionDetail{Value: d}

}



// errorProvider is a FeatureProvider that returns error results, simulating

// a transient failure such as a network timeout from Flipt.

type errorProvider struct {

	calls int

}



func (p *errorProvider) Metadata() openfeature.Metadata { return openfeature.Metadata{Name: "error"} }

func (p *errorProvider) Hooks() []openfeature.Hook      { return nil }



func (p *errorProvider) BooleanEvaluation(_ context.Context, _ string, defaultValue bool, _ openfeature.FlattenedContext) openfeature.BoolResolutionDetail {

	p.calls++

	return openfeature.BoolResolutionDetail{

		Value: defaultValue,

		ProviderResolutionDetail: openfeature.ProviderResolutionDetail{

			ResolutionError: openfeature.NewGeneralResolutionError("connection refused"),

			Reason:          openfeature.DefaultReason,

		},

	}

}



func (p *errorProvider) StringEvaluation(_ context.Context, _ string, defaultValue string, _ openfeature.FlattenedContext) openfeature.StringResolutionDetail {

	p.calls++

	return openfeature.StringResolutionDetail{

		Value: defaultValue,

		ProviderResolutionDetail: openfeature.ProviderResolutionDetail{

			ResolutionError: openfeature.NewGeneralResolutionError("connection refused"),

			Reason:          openfeature.DefaultReason,

		},

	}

}



func (p *errorProvider) FloatEvaluation(_ context.Context, _ string, d float64, _ openfeature.FlattenedContext) openfeature.FloatResolutionDetail {

	return openfeature.FloatResolutionDetail{Value: d}

}



func (p *errorProvider) IntEvaluation(_ context.Context, _ string, d int64, _ openfeature.FlattenedContext) openfeature.IntResolutionDetail {

	return openfeature.IntResolutionDetail{Value: d}

}



func (p *errorProvider) ObjectEvaluation(_ context.Context, _ string, d any, _ openfeature.FlattenedContext) openfeature.InterfaceResolutionDetail {

	return openfeature.InterfaceResolutionDetail{Value: d}

}



// recoveringProvider errors on the first call, then succeeds on subsequent calls.

type recoveringProvider struct {

	calls int

}



func (p *recoveringProvider) Metadata() openfeature.Metadata {

	return openfeature.Metadata{Name: "recovering"}

}

func (p *recoveringProvider) Hooks() []openfeature.Hook { return nil }



func (p *recoveringProvider) BooleanEvaluation(

	_ context.Context, _ string, defaultValue bool, _ openfeature.FlattenedContext,

) openfeature.BoolResolutionDetail {

	p.calls++

	if p.calls == 1 {

		return openfeature.BoolResolutionDetail{

			Value: defaultValue,

			ProviderResolutionDetail: openfeature.ProviderResolutionDetail{

				ResolutionError: openfeature.NewGeneralResolutionError("connection refused"),

				Reason:          openfeature.DefaultReason,

			},

		}

	}

	return openfeature.BoolResolutionDetail{

		Value:                    true,

		ProviderResolutionDetail: openfeature.ProviderResolutionDetail{Reason: openfeature.TargetingMatchReason},

	}

}



func (p *recoveringProvider) StringEvaluation(

	_ context.Context, _ string, defaultValue string, _ openfeature.FlattenedContext,

) openfeature.StringResolutionDetail {

	p.calls++

	return openfeature.StringResolutionDetail{Value: defaultValue}

}



func (p *recoveringProvider) FloatEvaluation(_ context.Context, _ string, d float64, _ openfeature.FlattenedContext) openfeature.FloatResolutionDetail {

	return openfeature.FloatResolutionDetail{Value: d}

}



func (p *recoveringProvider) IntEvaluation(_ context.Context, _ string, d int64, _ openfeature.FlattenedContext) openfeature.IntResolutionDetail {

	return openfeature.IntResolutionDetail{Value: d}

}



func (p *recoveringProvider) ObjectEvaluation(_ context.Context, _ string, d any, _ openfeature.FlattenedContext) openfeature.InterfaceResolutionDetail {

	return openfeature.InterfaceResolutionDetail{Value: d}

}



func newTestCachingProvider(t *testing.T, inner openfeature.FeatureProvider, ttl time.Duration, size int) *cachingProvider {

	t.Helper()

	p, err := newCachingProvider(inner, ttl, size)
@@ -248,3 +339,109 @@ func TestCachingProvider_StringMissAndHit(t *testing.T) { 
		})

	}

}



func TestCachingProvider_BoolErrorNotCached(t *testing.T) {

	tests := []struct {

		name          string

		callTwice     bool

		wantCallCount int

	}{

		{

			name:          "error on first call reaches inner provider",

			callTwice:     false,

			wantCallCount: 1,

		},

		{

			name:          "error on second call still reaches inner provider",

			callTwice:     true,

			wantCallCount: 2,

		},

	}



	for _, tc := range tests {

		t.Run(tc.name, func(t *testing.T) {

			inner := &errorProvider{}

			p := newTestCachingProvider(t, inner, 30*time.Second, 10)

			ctx := openfeature.FlattenedContext{}



			p.BooleanEvaluation(context.Background(), "test-flag", false, ctx)

			if tc.callTwice {

				p.BooleanEvaluation(context.Background(), "test-flag", false, ctx)

			}



			assert.Equal(t, tc.wantCallCount, inner.calls)

		})

	}

}



func TestCachingProvider_StringErrorNotCached(t *testing.T) {

	tests := []struct {

		name          string

		callTwice     bool

		wantCallCount int

	}{

		{

			name:          "error on first call reaches inner provider",

			callTwice:     false,

			wantCallCount: 1,

		},

		{

			name:          "error on second call still reaches inner provider",

			callTwice:     true,

			wantCallCount: 2,

		},

	}



	for _, tc := range tests {

		t.Run(tc.name, func(t *testing.T) {

			inner := &errorProvider{}

			p := newTestCachingProvider(t, inner, 30*time.Second, 10)

			ctx := openfeature.FlattenedContext{}



			p.StringEvaluation(context.Background(), "theme", "dark", ctx)

			if tc.callTwice {

				p.StringEvaluation(context.Background(), "theme", "dark", ctx)

			}



			assert.Equal(t, tc.wantCallCount, inner.calls)

		})

	}

}



func TestCachingProvider_BoolSuccessAfterErrorIsCached(t *testing.T) {

	tests := []struct {

		name          string

		totalCalls    int

		wantCallCount int

	}{

		{

			name:          "error on first call is not cached",

			totalCalls:    1,

			wantCallCount: 1,

		},

		{

			name:          "success on second call is cached",

			totalCalls:    2,

			wantCallCount: 2,

		},

		{

			name:          "third call served from cache after recovery",

			totalCalls:    3,

			wantCallCount: 2,

		},

	}



	for _, tc := range tests {

		t.Run(tc.name, func(t *testing.T) {

			inner := &recoveringProvider{}

			p := newTestCachingProvider(t, inner, 30*time.Second, 10)

			ctx := openfeature.FlattenedContext{}



			for range tc.totalCalls {

				p.BooleanEvaluation(context.Background(), "test-flag", false, ctx)

			}



			assert.Equal(t, tc.wantCallCount, inner.calls)

		})

	}

}