Skip to content

Commit 31a6893

Browse files
fabriziodemariaclaudenicklasl
authored
fix(go): Better error messaging for sticky rules (#110)
Co-authored-by: Claude <[email protected]> Co-authored-by: Nicklas Lundin <[email protected]>
1 parent 670c1ca commit 31a6893

File tree

6 files changed

+628
-83
lines changed

6 files changed

+628
-83
lines changed

openfeature-provider/go/confidence/local_resolver_provider.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,16 @@ func (p *LocalResolverProvider) ObjectEvaluation(
221221
// Get resolver API from factory
222222
resolverAPI := p.factory.GetSwapResolverAPI()
223223

224-
// Resolve flags
225-
response, err := resolverAPI.Resolve(request)
224+
// Create ResolveWithSticky request
225+
stickyRequest := &resolver.ResolveWithStickyRequest{
226+
ResolveRequest: request,
227+
MaterializationsPerUnit: make(map[string]*resolver.MaterializationMap),
228+
FailFastOnSticky: true,
229+
NotProcessSticky: false,
230+
}
231+
232+
// Resolve flags with sticky support
233+
stickyResponse, err := resolverAPI.ResolveWithSticky(stickyRequest)
226234
if err != nil {
227235
log.Printf("Failed to resolve flag '%s': %v", flagPath, err)
228236
return openfeature.InterfaceResolutionDetail{
@@ -234,6 +242,31 @@ func (p *LocalResolverProvider) ObjectEvaluation(
234242
}
235243
}
236244

245+
// Extract the actual resolve response from the sticky response
246+
var response *resolver.ResolveFlagsResponse
247+
switch result := stickyResponse.ResolveResult.(type) {
248+
case *resolver.ResolveWithStickyResponse_Success_:
249+
response = result.Success.Response
250+
case *resolver.ResolveWithStickyResponse_MissingMaterializations_:
251+
log.Printf("Missing materializations for flag '%s'", flagPath)
252+
return openfeature.InterfaceResolutionDetail{
253+
Value: defaultValue,
254+
ProviderResolutionDetail: openfeature.ProviderResolutionDetail{
255+
Reason: openfeature.ErrorReason,
256+
ResolutionError: openfeature.NewGeneralResolutionError("missing materializations"),
257+
},
258+
}
259+
default:
260+
log.Printf("Unexpected resolve result type for flag '%s'", flagPath)
261+
return openfeature.InterfaceResolutionDetail{
262+
Value: defaultValue,
263+
ProviderResolutionDetail: openfeature.ProviderResolutionDetail{
264+
Reason: openfeature.ErrorReason,
265+
ResolutionError: openfeature.NewGeneralResolutionError("unexpected resolve result"),
266+
},
267+
}
268+
}
269+
237270
// Check if flag was found
238271
if len(response.ResolvedFlags) == 0 {
239272
log.Printf("No active flag '%s' was found", flagPath)
Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
package confidence
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/open-feature/go-sdk/openfeature"
8+
adminv1 "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/proto/confidence/flags/admin/v1"
9+
iamv1 "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/proto/confidence/iam/v1"
10+
"github.com/tetratelabs/wazero"
11+
"google.golang.org/protobuf/proto"
12+
)
13+
14+
func TestLocalResolverProvider_ReturnsDefaultOnError(t *testing.T) {
15+
ctx := context.Background()
16+
runtime := wazero.NewRuntimeWithConfig(ctx, wazero.NewRuntimeConfig())
17+
defer runtime.Close(ctx)
18+
19+
// Create minimal state with wrong client secret
20+
state := &adminv1.ResolverState{
21+
Flags: []*adminv1.Flag{},
22+
ClientCredentials: []*iamv1.ClientCredential{
23+
{
24+
Credential: &iamv1.ClientCredential_ClientSecret_{
25+
ClientSecret: &iamv1.ClientCredential_ClientSecret{
26+
Secret: "wrong-secret",
27+
},
28+
},
29+
},
30+
},
31+
}
32+
stateBytes, _ := proto.Marshal(state)
33+
34+
flagLogger := NewNoOpWasmFlagLogger()
35+
swap, err := NewSwapWasmResolverApi(ctx, runtime, defaultWasmBytes, flagLogger, stateBytes, "test-account")
36+
if err != nil {
37+
t.Fatalf("Failed to create SwapWasmResolverApi: %v", err)
38+
}
39+
defer swap.Close(ctx)
40+
41+
factory := &LocalResolverFactory{
42+
resolverAPI: swap,
43+
}
44+
45+
// Use different client secret that won't match
46+
provider := NewLocalResolverProvider(factory, "test-secret")
47+
48+
evalCtx := openfeature.FlattenedContext{
49+
"user_id": "test-user",
50+
}
51+
52+
t.Run("StringEvaluation returns default on error", func(t *testing.T) {
53+
defaultValue := "default-value"
54+
result := provider.StringEvaluation(ctx, "non-existent-flag.field", defaultValue, evalCtx)
55+
56+
if result.Value != defaultValue {
57+
t.Errorf("Expected default value %v, got %v", defaultValue, result.Value)
58+
}
59+
60+
if result.Reason != openfeature.ErrorReason {
61+
t.Errorf("Expected ErrorReason, got %v", result.Reason)
62+
}
63+
64+
if result.ResolutionError.Error() == "" {
65+
t.Error("Expected ResolutionError to not be empty")
66+
}
67+
68+
t.Logf("✓ StringEvaluation correctly returned default value: %s", defaultValue)
69+
})
70+
}
71+
72+
func TestLocalResolverProvider_ReturnsCorrectValue(t *testing.T) {
73+
ctx := context.Background()
74+
runtime := wazero.NewRuntimeWithConfig(ctx, wazero.NewRuntimeConfig())
75+
defer runtime.Close(ctx)
76+
77+
// Load real test state
78+
testState := loadTestResolverState(t)
79+
testAcctID := loadTestAccountID(t)
80+
81+
flagLogger := NewNoOpWasmFlagLogger()
82+
swap, err := NewSwapWasmResolverApi(ctx, runtime, defaultWasmBytes, flagLogger, testState, testAcctID)
83+
if err != nil {
84+
t.Fatalf("Failed to create SwapWasmResolverApi: %v", err)
85+
}
86+
defer swap.Close(ctx)
87+
88+
factory := &LocalResolverFactory{
89+
resolverAPI: swap,
90+
}
91+
92+
// Use the correct client secret from test data
93+
provider := NewLocalResolverProvider(factory, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF")
94+
95+
evalCtx := openfeature.FlattenedContext{
96+
"visitor_id": "tutorial_visitor",
97+
}
98+
99+
t.Run("StringEvaluation returns correct variant value", func(t *testing.T) {
100+
defaultValue := "default-message"
101+
result := provider.StringEvaluation(ctx, "tutorial-feature.message", defaultValue, evalCtx)
102+
103+
// The exciting-welcome variant has a specific message
104+
expectedMessage := "We are very excited to welcome you to Confidence! This is a message from the tutorial flag."
105+
106+
if result.Value != expectedMessage {
107+
t.Errorf("Expected value '%s', got '%s'", expectedMessage, result.Value)
108+
}
109+
110+
if result.Reason != openfeature.TargetingMatchReason {
111+
t.Errorf("Expected TargetingMatchReason, got %v", result.Reason)
112+
}
113+
114+
if result.ResolutionError.Error() != "" {
115+
t.Errorf("Expected no error, got %v", result.ResolutionError)
116+
}
117+
})
118+
119+
t.Run("ObjectEvaluation returns correct variant structure", func(t *testing.T) {
120+
defaultValue := map[string]interface{}{
121+
"message": "default",
122+
"title": "default",
123+
}
124+
result := provider.ObjectEvaluation(ctx, "tutorial-feature", defaultValue, evalCtx)
125+
126+
if result.Value == nil {
127+
t.Fatal("Expected result value to not be nil")
128+
}
129+
130+
resultMap, ok := result.Value.(map[string]interface{})
131+
if !ok {
132+
t.Fatalf("Expected result value to be a map, got %T", result.Value)
133+
}
134+
135+
expectedMessage := "We are very excited to welcome you to Confidence! This is a message from the tutorial flag."
136+
expectedTitle := "Welcome to Confidence!"
137+
138+
if resultMap["message"] != expectedMessage {
139+
t.Errorf("Expected message '%s', got '%v'", expectedMessage, resultMap["message"])
140+
}
141+
142+
if resultMap["title"] != expectedTitle {
143+
t.Errorf("Expected title '%s', got '%v'", expectedTitle, resultMap["title"])
144+
}
145+
146+
if result.Reason != openfeature.TargetingMatchReason {
147+
t.Errorf("Expected TargetingMatchReason, got %v", result.Reason)
148+
}
149+
150+
if result.ResolutionError.Error() != "" {
151+
t.Errorf("Expected no error, got %v", result.ResolutionError)
152+
}
153+
})
154+
}
155+
156+
func TestLocalResolverProvider_MissingMaterializations(t *testing.T) {
157+
ctx := context.Background()
158+
runtime := wazero.NewRuntimeWithConfig(ctx, wazero.NewRuntimeConfig())
159+
defer runtime.Close(ctx)
160+
161+
t.Run("Provider returns resolved value for flag without sticky rules", func(t *testing.T) {
162+
// Load real test state
163+
testState := loadTestResolverState(t)
164+
testAcctID := loadTestAccountID(t)
165+
166+
flagLogger := NewNoOpWasmFlagLogger()
167+
swap, err := NewSwapWasmResolverApi(ctx, runtime, defaultWasmBytes, flagLogger, testState, testAcctID)
168+
if err != nil {
169+
t.Fatalf("Failed to create SwapWasmResolverApi: %v", err)
170+
}
171+
defer swap.Close(ctx)
172+
173+
factory := &LocalResolverFactory{
174+
resolverAPI: swap,
175+
}
176+
177+
provider := NewLocalResolverProvider(factory, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF")
178+
179+
evalCtx := openfeature.FlattenedContext{
180+
"visitor_id": "tutorial_visitor",
181+
}
182+
183+
// The tutorial-feature flag in the test data doesn't have materialization requirements
184+
// so resolving with empty materializations should succeed
185+
defaultValue := "default"
186+
result := provider.StringEvaluation(ctx, "tutorial-feature.message", defaultValue, evalCtx)
187+
188+
if result.ResolutionError.Error() != "" {
189+
t.Errorf("Expected successful resolve for flag without sticky rules, got error: %v", result.ResolutionError)
190+
}
191+
192+
if result.Value == defaultValue {
193+
t.Error("Expected resolved value, got default value")
194+
}
195+
196+
if result.Reason != openfeature.TargetingMatchReason {
197+
t.Errorf("Expected TargetingMatchReason, got %v", result.Reason)
198+
}
199+
})
200+
201+
t.Run("Provider returns missing materializations error message", func(t *testing.T) {
202+
// Create state with a flag that requires materializations
203+
stickyState := createStateWithStickyFlag()
204+
accountId := "test-account"
205+
206+
flagLogger := NewNoOpWasmFlagLogger()
207+
swap, err := NewSwapWasmResolverApi(ctx, runtime, defaultWasmBytes, flagLogger, stickyState, accountId)
208+
if err != nil {
209+
t.Fatalf("Failed to create SwapWasmResolverApi: %v", err)
210+
}
211+
defer swap.Close(ctx)
212+
213+
factory := &LocalResolverFactory{
214+
resolverAPI: swap,
215+
}
216+
217+
provider := NewLocalResolverProvider(factory, "test-secret")
218+
219+
evalCtx := openfeature.FlattenedContext{
220+
"user_id": "test-user-123",
221+
}
222+
223+
defaultValue := false
224+
result := provider.BooleanEvaluation(ctx, "sticky-test-flag.enabled", defaultValue, evalCtx)
225+
226+
if result.Value != defaultValue {
227+
t.Errorf("Expected default value %v when materializations missing, got %v", defaultValue, result.Value)
228+
}
229+
230+
if result.Reason != openfeature.ErrorReason {
231+
t.Errorf("Expected ErrorReason when materializations missing, got %v", result.Reason)
232+
}
233+
234+
if result.ResolutionError.Error() == "" {
235+
t.Error("Expected ResolutionError when materializations missing")
236+
}
237+
238+
expectedErrorMsg := "missing materializations"
239+
if result.ResolutionError.Error() != "GENERAL: missing materializations" {
240+
t.Errorf("Expected error message 'GENERAL: %s', got: %v", expectedErrorMsg, result.ResolutionError)
241+
}
242+
})
243+
}

openfeature-provider/go/confidence/resolver_api.go

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,11 @@ type ResolverApi struct {
2727
runtime wazero.Runtime
2828

2929
// WASM exports
30-
wasmMsgAlloc api.Function
31-
wasmMsgFree api.Function
32-
wasmMsgGuestSetResolverState api.Function
33-
wasmMsgGuestFlushLogs api.Function
34-
wasmMsgGuestResolve api.Function
35-
wasmMsgGuestResolveSimple api.Function
30+
wasmMsgAlloc api.Function
31+
wasmMsgFree api.Function
32+
wasmMsgGuestSetResolverState api.Function
33+
wasmMsgGuestFlushLogs api.Function
34+
wasmMsgGuestResolveWithSticky api.Function
3635

3736
// Flag logger for writing logs
3837
flagLogger WasmFlagLogger
@@ -117,23 +116,23 @@ func NewResolverApiFromCompiled(ctx context.Context, runtime wazero.Runtime, com
117116
wasmMsgFree := instance.ExportedFunction("wasm_msg_free")
118117
wasmMsgGuestSetResolverState := instance.ExportedFunction("wasm_msg_guest_set_resolver_state")
119118
wasmMsgGuestFlushLogs := instance.ExportedFunction("wasm_msg_guest_flush_logs")
120-
wasmMsgGuestResolve := instance.ExportedFunction("wasm_msg_guest_resolve")
119+
wasmMsgGuestResolveWithSticky := instance.ExportedFunction("wasm_msg_guest_resolve_with_sticky")
121120

122-
if wasmMsgAlloc == nil || wasmMsgFree == nil || wasmMsgGuestSetResolverState == nil || wasmMsgGuestFlushLogs == nil || wasmMsgGuestResolve == nil {
121+
if wasmMsgAlloc == nil || wasmMsgFree == nil || wasmMsgGuestSetResolverState == nil || wasmMsgGuestFlushLogs == nil || wasmMsgGuestResolveWithSticky == nil {
123122
panic("Required WASM exports not found")
124123
}
125124

126125
return &ResolverApi{
127-
instance: instance,
128-
module: compiledModule,
129-
runtime: runtime,
130-
wasmMsgAlloc: wasmMsgAlloc,
131-
wasmMsgFree: wasmMsgFree,
132-
wasmMsgGuestSetResolverState: wasmMsgGuestSetResolverState,
133-
wasmMsgGuestFlushLogs: wasmMsgGuestFlushLogs,
134-
wasmMsgGuestResolve: wasmMsgGuestResolve,
135-
flagLogger: flagLogger,
136-
firstResolve: true,
126+
instance: instance,
127+
module: compiledModule,
128+
runtime: runtime,
129+
wasmMsgAlloc: wasmMsgAlloc,
130+
wasmMsgFree: wasmMsgFree,
131+
wasmMsgGuestSetResolverState: wasmMsgGuestSetResolverState,
132+
wasmMsgGuestFlushLogs: wasmMsgGuestFlushLogs,
133+
wasmMsgGuestResolveWithSticky: wasmMsgGuestResolveWithSticky,
134+
flagLogger: flagLogger,
135+
firstResolve: true,
137136
}
138137
}
139138

@@ -240,8 +239,8 @@ func (r *ResolverApi) SetResolverState(state []byte, accountId string) error {
240239
// ErrInstanceClosed is returned when the WASM instance is being closed/swapped
241240
var ErrInstanceClosed = errors.New("WASM instance is closed or being replaced")
242241

243-
// Resolve resolves flags using the WASM module
244-
func (r *ResolverApi) Resolve(request *resolver.ResolveFlagsRequest) (*resolver.ResolveFlagsResponse, error) {
242+
// ResolveWithSticky resolves flags with sticky targeting support using the WASM module
243+
func (r *ResolverApi) ResolveWithSticky(request *resolver.ResolveWithStickyRequest) (*resolver.ResolveWithStickyResponse, error) {
245244
// Acquire lock first, then check isClosing flag to prevent race condition
246245
// where instance could be marked as closing between check and lock acquisition.
247246
// If closing, return immediately with ErrInstanceClosed to prevent using stale instance.
@@ -257,17 +256,17 @@ func (r *ResolverApi) Resolve(request *resolver.ResolveFlagsRequest) (*resolver.
257256
reqPtr := r.transferRequest(request)
258257

259258
// Call the WASM function
260-
results, err := r.wasmMsgGuestResolve.Call(ctx, uint64(reqPtr))
259+
results, err := r.wasmMsgGuestResolveWithSticky.Call(ctx, uint64(reqPtr))
261260
if err != nil {
262-
return nil, fmt.Errorf("failed to call wasm_msg_guest_resolve: %w", err)
261+
return nil, fmt.Errorf("failed to call wasm_msg_guest_resolve_with_sticky: %w", err)
263262
}
264263

265264
// Consume the response
266265
respPtr := uint32(results[0])
267-
response := &resolver.ResolveFlagsResponse{}
266+
response := &resolver.ResolveWithStickyResponse{}
268267
err = r.consumeResponse(respPtr, response)
269268
if err != nil {
270-
log.Printf("Resolve failed with error: %v", err)
269+
log.Printf("ResolveWithSticky failed with error: %v", err)
271270
return nil, err
272271
}
273272

0 commit comments

Comments
 (0)