Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (o *OptimizelyClient) decide(userContext *OptimizelyUserContext, key string
}

if err != nil {
o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature %q: %s`, key, err))
return o.handleDecisionServiceError(err, "decide", key, *userContext)
}

if featureDecision.Variation != nil {
Expand Down Expand Up @@ -1244,3 +1244,9 @@ func (o *OptimizelyClient) getDecisionVariableMap(feature entities.Feature, vari
func isNil(v interface{}) bool {
return v == nil || (reflect.ValueOf(v).Kind() == reflect.Ptr && reflect.ValueOf(v).IsNil())
}

func (o *OptimizelyClient) handleDecisionServiceError(err error, methodName, key string, userContext OptimizelyUserContext) OptimizelyDecision {
o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature %q in %s: %s`, key, methodName, err))

return NewErrorDecision(key, userContext, err)
}
3 changes: 1 addition & 2 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1682,8 +1682,7 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) {
ConfigManager: mockConfigManager,
DecisionService: mockDecisionService,
logger: logging.GetLogger("", ""),
tracer: &MockTracer{},
}
tracer: &MockTracer{}}

_, decision, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext)
assert.Equal(t, expectedFeatureDecision, decision)
Expand Down
17 changes: 6 additions & 11 deletions pkg/cmab/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,11 @@ func NewDefaultCmabClient(options ClientOptions) *DefaultCmabClient {

// FetchDecision fetches a decision from the CMAB API
func (c *DefaultCmabClient) FetchDecision(
ctx context.Context,
ruleID string,
userID string,
attributes map[string]interface{},
cmabUUID string,
) (string, error) {
// If no context is provided, create a background context
if ctx == nil {
ctx = context.Background()
}

// Create the URL
url := fmt.Sprintf(CMABPredictionEndpoint, ruleID)
Expand Down Expand Up @@ -171,18 +166,18 @@ func (c *DefaultCmabClient) FetchDecision(

// If no retry config, just do a single fetch
if c.retryConfig == nil {
return c.doFetch(ctx, url, bodyBytes)
return c.doFetch(context.Background(), url, bodyBytes)
}

// Retry sending request with exponential backoff
for i := 0; i <= c.retryConfig.MaxRetries; i++ {
// Check if context is done
if ctx.Err() != nil {
return "", fmt.Errorf("context canceled or timed out: %w", ctx.Err())
if context.Background().Err() != nil {
return "", fmt.Errorf("context canceled or timed out: %w", context.Background().Err())
}

// Make the request
result, err := c.doFetch(ctx, url, bodyBytes)
result, err := c.doFetch(context.Background(), url, bodyBytes)
if err == nil {
return result, nil
}
Expand All @@ -204,8 +199,8 @@ func (c *DefaultCmabClient) FetchDecision(

// Wait for backoff duration with context awareness
select {
case <-ctx.Done():
return "", fmt.Errorf("context canceled or timed out during backoff: %w", ctx.Err())
case <-context.Background().Done():
return "", fmt.Errorf("context canceled or timed out during backoff: %w", context.Background().Err())
case <-time.After(backoffDuration):
// Continue with retry
}
Expand Down
73 changes: 9 additions & 64 deletions pkg/cmab/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package cmab

import (
"context"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -141,10 +140,7 @@ func TestDefaultCmabClient_FetchDecision(t *testing.T) {
"null_attr": nil,
}

// Create a context for the request
ctx := context.Background()

variationID, err := client.FetchDecision(ctx, "rule456", "user123", attributes, "test-uuid")
variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
assert.NoError(t, err)
Expand Down Expand Up @@ -223,7 +219,7 @@ func TestDefaultCmabClient_FetchDecision_WithRetry(t *testing.T) {
}

startTime := time.Now()
variationID, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")
duration := time.Since(startTime)

// Verify results
Expand Down Expand Up @@ -270,7 +266,7 @@ func TestDefaultCmabClient_FetchDecision_ExhaustedRetries(t *testing.T) {
"isMobile": true,
}

variationID, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
assert.Error(t, err)
Expand Down Expand Up @@ -309,7 +305,7 @@ func TestDefaultCmabClient_FetchDecision_NoRetryConfig(t *testing.T) {
"browser": "chrome",
}

_, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
_, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
assert.Error(t, err)
Expand Down Expand Up @@ -372,7 +368,7 @@ func TestDefaultCmabClient_FetchDecision_InvalidResponse(t *testing.T) {
"browser": "chrome",
}

_, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
_, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
assert.Error(t, err)
Expand Down Expand Up @@ -416,7 +412,7 @@ func TestDefaultCmabClient_FetchDecision_NetworkErrors(t *testing.T) {
"browser": "chrome",
}

_, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
_, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
assert.Error(t, err)
Expand Down Expand Up @@ -476,7 +472,7 @@ func TestDefaultCmabClient_ExponentialBackoff(t *testing.T) {
"browser": "chrome",
}

variationID, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
require.NoError(t, err)
Expand Down Expand Up @@ -564,7 +560,7 @@ func TestDefaultCmabClient_LoggingBehavior(t *testing.T) {
"browser": "chrome",
}

_, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid")
_, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")
assert.NoError(t, err)

// Verify log messages
Expand Down Expand Up @@ -627,10 +623,7 @@ func TestDefaultCmabClient_NonSuccessStatusCode(t *testing.T) {
"browser": "chrome",
}

// Create a context for the request
ctx := context.Background()

variationID, err := client.FetchDecision(ctx, "rule456", "user123", attributes, "test-uuid")
variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid")

// Verify results
assert.Error(t, err, "Expected error for non-success status code")
Expand All @@ -640,51 +633,3 @@ func TestDefaultCmabClient_NonSuccessStatusCode(t *testing.T) {
})
}
}

func TestDefaultCmabClient_FetchDecision_ContextCancellation(t *testing.T) {
// Setup test server that delays response
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Sleep to simulate a slow response
time.Sleep(500 * time.Millisecond)

w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
response := Response{
Predictions: []Prediction{
{
VariationID: "var123",
},
},
}
json.NewEncoder(w).Encode(response)
}))
defer server.Close()

// Create client with custom endpoint
client := NewDefaultCmabClient(ClientOptions{
HTTPClient: &http.Client{
Timeout: 5 * time.Second,
},
})

// Override the endpoint for testing
originalEndpoint := CMABPredictionEndpoint
CMABPredictionEndpoint = server.URL + "/%s"
defer func() { CMABPredictionEndpoint = originalEndpoint }()

// Create a context with a short timeout
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()

// Test fetch decision with a context that will time out
attributes := map[string]interface{}{
"browser": "chrome",
}

_, err := client.FetchDecision(ctx, "rule456", "user123", attributes, "test-uuid")

// Verify that we got a context deadline exceeded error
assert.Error(t, err)
assert.Contains(t, err.Error(), "context")
assert.Contains(t, err.Error(), "deadline exceeded")
}
Loading
Loading