Increase AppSearch read timeout since it appears that p99 > 500ms

See https://screenshot.googleplex.com/6GsEJoXt4gSAUA2, it shows that the call to AppSearch GlobalSearch is currently taking just over 500ms. This will result in a timeout and no data being returned. Bumping up the timeout to lessen the chances of that happening.

Also found some other instances where we were logging the original thrown exception to logcat and re-throwing a generic RuntimeException without passing in the original as the cause, fixing those as well.

Bug: 316920565
Test: atest
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:4aa249c07be49a08f96f793b17d0e4a48fd7b12f)
Merged-In: I6f912308c779cedeb4969dd4e2c7ca3187d9beb7
Change-Id: I6f912308c779cedeb4969dd4e2c7ca3187d9beb7

5 files changed, 25 insertions, 14 deletions

diff --git a/adservices/service-core/java/com/android/adservices/service/Flags.java b/adservices/service-core/java/com/android/adservices/service/Flags.java
index 6f3264e1a..57dd4300e 100644
--- a/adservices/service-core/java/com/android/adservices/service/Flags.java
+++ b/adservices/service-core/java/com/android/adservices/service/Flags.java
@@ -4517,7 +4517,7 @@ public interface Flags extends CommonFlags {
     }
 
     /** Default value of the timeout for AppSearch read operations */
-    int DEFAULT_APPSEARCH_READ_TIMEOUT_MS = 500;
+    int DEFAULT_APPSEARCH_READ_TIMEOUT_MS = 750;
 
     /**
      * Gets the value of the timeout for AppSearch read operations, in milliseconds.
diff --git a/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchDao.java b/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchDao.java
index cd52dbd72..48d27d1c5 100644
--- a/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchDao.java
+++ b/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchDao.java
@@ -305,13 +305,17 @@ class AppSearchDao {
                                         // If we get failures in schemaResponse then we cannot try
                                         // to write.
                                         if (!setSchemaResponse.getMigrationFailures().isEmpty()) {
+                                            MigrationFailure failure =
+                                                    setSchemaResponse.getMigrationFailures().get(0);
                                             LogUtil.e(
                                                     "SetSchemaResponse migration failure: "
-                                                            + setSchemaResponse
-                                                                    .getMigrationFailures()
-                                                                    .get(0));
-                                            throw new RuntimeException(
-                                                    ERROR_MESSAGE_APPSEARCH_FAILURE);
+                                                            + failure);
+                                            String message =
+                                                    String.format(
+                                                            "%s Migration failure: %s",
+                                                            ERROR_MESSAGE_APPSEARCH_FAILURE,
+                                                            failure.getAppSearchResult());
+                                            throw new RuntimeException(message);
                                         }
                                         // The database knows about this schemaType and write can
                                         // occur.
@@ -323,10 +327,10 @@ class AppSearchDao {
                                     executor);
             return deleteFuture;
         } catch (AppSearchException e) {
-            LogUtil.e("Cannot instantiate AppSearch database: " + e.getMessage());
+            LogUtil.e(e, "Cannot instantiate AppSearch database");
+            return FluentFuture.from(
+                    Futures.immediateFailedFuture(
+                            new RuntimeException(ERROR_MESSAGE_APPSEARCH_FAILURE, e)));
         }
-        return FluentFuture.from(
-                Futures.immediateFailedFuture(
-                        new RuntimeException(ERROR_MESSAGE_APPSEARCH_FAILURE)));
     }
 }
diff --git a/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorker.java b/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorker.java
index b282cfefa..85e7baf07 100644
--- a/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorker.java
+++ b/adservices/service-core/java/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorker.java
@@ -105,7 +105,7 @@ public final class AppSearchMeasurementRollbackWorker implements MeasurementRoll
             LogUtil.d("Wrote measurement rollback data to AppSearch: %s", dao);
         } catch (InterruptedException | TimeoutException | ExecutionException e) {
             LogUtil.e(e, "Failed to write measurement rollback to AppSearch");
-            throw new RuntimeException(ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE);
+            throw new RuntimeException(ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE, e);
         } finally {
             READ_WRITE_LOCK.writeLock().unlock();
         }
@@ -137,7 +137,7 @@ public final class AppSearchMeasurementRollbackWorker implements MeasurementRoll
             LogUtil.d("Deleted MeasurementRollback data from AppSearch for: %s", storageIdentifier);
         } catch (InterruptedException | TimeoutException | ExecutionException e) {
             LogUtil.e(e, "Failed to delete MeasurementRollback data in AppSearch");
-            throw new RuntimeException(ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE);
+            throw new RuntimeException(ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE, e);
         } finally {
             READ_WRITE_LOCK.writeLock().unlock();
         }
diff --git a/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchDaoTest.java b/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchDaoTest.java
index 116891316..5627ff9fb 100644
--- a/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchDaoTest.java
+++ b/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchDaoTest.java
@@ -353,7 +353,8 @@ public class AppSearchDaoTest {
         when(mockSession.setSchemaAsync(any(SetSchemaRequest.class)))
                 .thenReturn(Futures.immediateFuture(mockResponse));
 
-        AppSearchResult mockResult = Mockito.mock(AppSearchResult.class);
+        AppSearchResult<String> mockResult =
+                AppSearchResult.newFailedResult(AppSearchResult.RESULT_INVALID_ARGUMENT, "test");
         SetSchemaResponse.MigrationFailure failure =
                 new SetSchemaResponse.MigrationFailure(
                         /* namespace= */ TEST,
@@ -374,7 +375,8 @@ public class AppSearchDaoTest {
         assertThat(e.getMessage())
                 .isEqualTo(
                         "java.lang.RuntimeException: "
-                                + ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE);
+                                + ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE
+                                + " Migration failure: [FAILURE(3)]: test");
     }
 
     @Test
diff --git a/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorkerTest.java b/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorkerTest.java
index b135b209f..e8137c71c 100644
--- a/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorkerTest.java
+++ b/adservices/tests/unittest/service-core/appsearch/src/com/android/adservices/service/appsearch/AppSearchMeasurementRollbackWorkerTest.java
@@ -62,6 +62,7 @@ import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 @SmallTest
 public class AppSearchMeasurementRollbackWorkerTest {
@@ -157,6 +158,8 @@ public class AppSearchMeasurementRollbackWorkerTest {
                         RuntimeException.class,
                         () -> mWorker.clearAdServicesDeletionOccurred("mock_row_id"));
         assertThat(e).hasMessageThat().contains(ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE);
+        assertThat(e).hasCauseThat().isNotNull();
+        assertThat(e).hasCauseThat().isInstanceOf(TimeoutException.class);
     }
 
     @Test
@@ -276,6 +279,8 @@ public class AppSearchMeasurementRollbackWorkerTest {
                                 spyWorker.recordAdServicesDeletionOccurred(
                                         AdServicesManager.MEASUREMENT_DELETION, APEX_VERSION));
         assertThat(e).hasMessageThat().contains(ConsentConstants.ERROR_MESSAGE_APPSEARCH_FAILURE);
+        assertThat(e).hasCauseThat().isNotNull();
+        assertThat(e).hasCauseThat().isInstanceOf(TimeoutException.class);
     }
 
     @Test