[FSSDK-12111] update cmab error handling

.github/workflows/android.yml

@@ -81,7 +81,18 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        api-level: [21, 25, 26, 29]
+        # 21 is too old (not properly supported by github)
+        # mockito version is not working in 30+
+        include:
+          - api-level: 25
+            arch: x86
+            target: google_apis
+          - api-level: 27
+            arch: x86
+            target: google_apis
+          - api-level: 29
+            arch: x86_64
+            target: default
     steps:
       - name: checkout
         uses: actions/checkout@v4
@@ -113,14 +124,15 @@ jobs:
             ~/.android/avd/*
             ~/.android/adb*
             ~/.android/debug.keystore
-          key: avd-${{ matrix.api-level }}
+          key: avd-${{ matrix.api-level }}-${{ matrix.arch }}-${{ matrix.target }}
 
       - name: create AVD and generate snapshot for caching
         if: steps.avd-cache.outputs.cache-hit != 'true'
         uses: reactivecircus/android-emulator-runner@v2
         with:
           api-level: ${{ matrix.api-level }}
-          # arch: arm64-v8a # Specify ARM architecture 
+          arch: ${{ matrix.arch }}
+          target: ${{ matrix.target }}
           force-avd-creation: false
           emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
           disable-animations: false
@@ -130,6 +142,8 @@ jobs:
         uses: reactivecircus/android-emulator-runner@v2
         with:
           api-level: ${{ matrix.api-level }}
+          arch: ${{ matrix.arch }}
+          target: ${{ matrix.target }}
           force-avd-creation: false
           emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
           disable-animations: true

.gitignore

@@ -10,7 +10,9 @@
 /build
 /captures
 libs/
+fsc-test-libs/
 values.gradle
 jacoco.exec
 .vscode/
+.settings/
 

android-sdk/build.gradle

@@ -73,8 +73,8 @@ dependencies {
     api project(':user-profile')
     api project(':odp')
     // Enable using a local core-api jar for testing when the 'useLocalJars' property is specified
-    if (project.hasProperty('useLocalJars') && file('../libs/core-api.jar').exists()) {
-        api files('../libs/core-api.jar')
+    if (project.hasProperty('useLocalJars') && file('../fsc-test-libs/core-api.jar').exists()) {
+        api files('../fsc-test-libs/core-api.jar')
     } else {
         api ("com.optimizely.ab:core-api:$java_core_ver") {
             exclude group: 'com.google.code.findbugs'

android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClient.kt

@@ -107,6 +107,12 @@ open class DefaultCmabClient : CmabClient {
                     logger.error(errorMessage)
                     throw CmabFetchException(errorMessage)
                 }
+            } catch (e: CmabInvalidResponseException) {
+                // Propagate validation exceptions as-is
+                throw e
+            } catch (e: CmabFetchException) {
+                // Propagate fetch exceptions as-is
+                throw e
             } catch (e: Exception) {
                 logger.debug("Failed to fetch CMAB decision for ruleId={} and userId={}", ruleId, userId);
                 val errorMessage: String =
@@ -123,7 +129,17 @@ open class DefaultCmabClient : CmabClient {
                 }
             }
         }
-        return client.execute(request, REQUEST_BACKOFF_TIMEOUT, REQUEST_RETRIES_POWER)
+        val result = client.execute(request, REQUEST_BACKOFF_TIMEOUT, REQUEST_RETRIES_POWER)
+
+        // Required to throw exception on any error so CmabService can return cmab error decision.
+        // Null result means CMAB fetch failed - throw exception.
+        if (result == null) {
+            val errorMessage = String.format(cmabClientHelper.cmabFetchFailed, "Client returned null - request failed")
+            logger.error(errorMessage)
+            throw CmabFetchException(errorMessage)
+        }
+
+        return result
     }
 
     companion object {

android-sdk/src/test/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClientTest.java

@@ -15,6 +15,8 @@
 package com.optimizely.ab.android.sdk.cmab;
 
 import com.optimizely.ab.android.shared.Client;
+import com.optimizely.ab.cmab.client.CmabFetchException;
+import com.optimizely.ab.cmab.client.CmabInvalidResponseException;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -88,15 +90,20 @@ public void testConstructorWithContextAndHelper() {
     }
 
     @Test
-    public void testFetchDecisionSuccess() throws Exception {
+    public void testFetchDecisionSuccess() {
         HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class);
         ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class);
 
-        String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}";
-        when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
-        when(mockUrlConnection.getResponseCode()).thenReturn(200);
-        when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson);
-        when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+        try {
+            String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}";
+            when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
+            when(mockUrlConnection.getResponseCode()).thenReturn(200);
+            when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson);
+            when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+        } catch (IOException e) {
+            // Never happens with mocked connection
+        }
+
         when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
             Client.Request<String> request = invocation.getArgument(0);
             return request.execute();
@@ -120,13 +127,18 @@ public void testFetchDecisionSuccess() throws Exception {
 
         verify(mockUrlConnection).setConnectTimeout(10*1000);
         verify(mockUrlConnection).setReadTimeout(60*1000);
-        verify(mockUrlConnection).setRequestMethod("POST");
+        try {
+            verify(mockUrlConnection).setRequestMethod("POST");
+        } catch (Exception e) {
+            // Never happens - verify() doesn't actually invoke the method
+        }
         verify(mockUrlConnection).setRequestProperty("content-type", "application/json");
         verify(mockUrlConnection).setDoOutput(true);
     }
 
-    @Test
-    public void testFetchDecisionConnectionFailure() throws Exception {
+    @Test(expected = CmabFetchException.class)
+    public void testFetchDecisionConnectionFailure() {
+        // When openConnection returns null, should throw CmabFetchException
         when(mockClient.openConnection(any(URL.class))).thenReturn(null);
         when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
             Client.Request<String> request = invocation.getArgument(0);
@@ -135,20 +147,164 @@ public void testFetchDecisionConnectionFailure() throws Exception {
 
         mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper);
 
-        String result = mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
-        assertNull(result);
+        // Should throw CmabFetchException when connection fails to open
+        mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
+    }
+
+    @Test(expected = CmabFetchException.class)
+    public void testFetchDecisionThrowsExceptionOn500Error() {
+        HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class);
+        ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class);
+
+        try {
+            when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
+            when(mockUrlConnection.getResponseCode()).thenReturn(500);
+            when(mockUrlConnection.getResponseMessage()).thenReturn("Internal Server Error");
+            when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+        } catch (IOException e) {
+            // Never happens with mocked connection
+        }
+
+        when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
+            Client.Request<String> request = invocation.getArgument(0);
+            return request.execute();
+        });
+
+        doReturn("{\"user_id\":\"test-user-456\"}")
+            .when(mockCmabClientHelper)
+            .buildRequestJson(any(), any(), any(), any());
+
+        mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper);
+
+        // Should throw CmabFetchException for 500 error
+        mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
+    }
+
+    @Test(expected = CmabFetchException.class)
+    public void testFetchDecisionThrowsExceptionOn400Error() {
+        HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class);
+        ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class);
+
+        try {
+            when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
+            when(mockUrlConnection.getResponseCode()).thenReturn(400);
+            when(mockUrlConnection.getResponseMessage()).thenReturn("Bad Request");
+            when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+        } catch (IOException e) {
+            // Never happens with mocked connection
+        }
+
+        when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
+            Client.Request<String> request = invocation.getArgument(0);
+            return request.execute();
+        });
+
+        doReturn("{\"user_id\":\"test-user-456\"}")
+            .when(mockCmabClientHelper)
+            .buildRequestJson(any(), any(), any(), any());
+
+        mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper);
+
+        // Should throw CmabFetchException for 400 error
+        mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
+    }
+
+    @Test(expected = CmabFetchException.class)
+    public void testFetchDecisionThrowsExceptionOnNetworkError() {
+        HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class);
+        ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class);
+
+        try {
+            when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
+            when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+            doThrow(new IOException("Network error")).when(mockUrlConnection).getResponseCode();
+        } catch (IOException e) {
+            // Never happens with mocked connection
+        }
+
+        when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
+            Client.Request<String> request = invocation.getArgument(0);
+            return request.execute();
+        });
+
+        doReturn("{\"user_id\":\"test-user-456\"}")
+            .when(mockCmabClientHelper)
+            .buildRequestJson(any(), any(), any(), any());
+
+        mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper);
+
+        // Should throw CmabFetchException when network IOException occurs
+        mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
+    }
+
+    @Test(expected = CmabInvalidResponseException.class)
+    public void testFetchDecisionThrowsExceptionOnInvalidJson() {
+        HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class);
+        ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class);
+
+        try {
+            String invalidResponseJson = "{\"invalid\":\"response\"}";
+            when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
+            when(mockUrlConnection.getResponseCode()).thenReturn(200);
+            when(mockClient.readStream(mockUrlConnection)).thenReturn(invalidResponseJson);
+            when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+        } catch (IOException e) {
+            // Never happens with mocked connection
+        }
+
+        when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
+            Client.Request<String> request = invocation.getArgument(0);
+            return request.execute();
+        });
+
+        doReturn("{\"user_id\":\"test-user-456\"}")
+            .when(mockCmabClientHelper)
+            .buildRequestJson(any(), any(), any(), any());
+        doReturn(false)  // Invalid response
+            .when(mockCmabClientHelper)
+            .validateResponse(any());
+
+        mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper);
+
+        // Should throw CmabInvalidResponseException when response validation fails
+        mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
     }
 
     @Test
-    public void testRetryOnFailureWithRetryBackoff() throws Exception {
-        when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn(null);
+    public void testRetryConfigurationPassedToClient() {
+        HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class);
+        ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class);
+
+        try {
+            String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}";
+            when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
+            when(mockUrlConnection.getResponseCode()).thenReturn(200);
+            when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson);
+            when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
+        } catch (IOException e) {
+            // Never happens with mocked connection
+        }
+
+        when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
+            Client.Request<String> request = invocation.getArgument(0);
+            return request.execute();
+        });
+
+        doReturn("{\"user_id\":\"test-user-456\"}")
+            .when(mockCmabClientHelper)
+            .buildRequestJson(any(), any(), any(), any());
+        doReturn(true)
+            .when(mockCmabClientHelper)
+            .validateResponse(any());
+        doReturn("variation_1")
+            .when(mockCmabClientHelper)
+            .parseVariationId(any());
 
         mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper);
 
-        String result = mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
-        assertNull(result);
+        mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
 
-        // Verify the retry configuration matches our constants
+        // Verify the retry configuration is passed to client.execute()
         verify(mockClient).execute(any(Client.Request.class), eq(DefaultCmabClient.REQUEST_BACKOFF_TIMEOUT), eq(DefaultCmabClient.REQUEST_RETRIES_POWER));
         assertEquals("REQUEST_BACKOFF_TIMEOUT should be 1", 1, DefaultCmabClient.REQUEST_BACKOFF_TIMEOUT);
         assertEquals("REQUEST_RETRIES_POWER should be 1", 1, DefaultCmabClient.REQUEST_RETRIES_POWER);

shared/build.gradle

@@ -46,8 +46,8 @@ android {
 
 dependencies {
     // Enable using a local core-api jar for testing when the 'useLocalJars' property is specified
-    if (project.hasProperty('useLocalJars') && file('../libs/core-api.jar').exists()) {
-        api files('../libs/core-api.jar')
+    if (project.hasProperty('useLocalJars') && file('../fsc-test-libs/core-api.jar').exists()) {
+        api files('../fsc-test-libs/core-api.jar')
     } else {
         api ("com.optimizely.ab:core-api:$java_core_ver") {
             exclude group: 'com.google.code.findbugs'