Merge "Remove ParcelableException from AppSearch." into sc-dev

apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java

@@ -19,9 +19,9 @@ package android.app.appsearch;
 import android.annotation.CallbackExecutor;
 import android.annotation.NonNull;
 import android.annotation.UserIdInt;
+import android.app.appsearch.exceptions.AppSearchException;
 import android.app.appsearch.util.SchemaMigrationUtil;
 import android.os.Bundle;
-import android.os.ParcelableException;
 import android.os.RemoteException;
 import android.os.SystemClock;
 import android.util.ArrayMap;
@@ -274,12 +274,14 @@ public final class AppSearchSession implements Closeable {
             mService.putDocuments(mPackageName, mDatabaseName, documentBundles, mUserId,
                     /*binderCallStartTimeMillis=*/ SystemClock.elapsedRealtime(),
                     new IAppSearchBatchResultCallback.Stub() {
+                        @Override
                         public void onResult(AppSearchBatchResult result) {
                             executor.execute(() -> callback.onResult(result));
                         }
 
-                        public void onSystemError(ParcelableException exception) {
-                            executor.execute(() -> callback.onSystemError(exception.getCause()));
+                        @Override
+                        public void onSystemError(AppSearchResult result) {
+                            executor.execute(() -> sendSystemErrorToCallback(result, callback));
                         }
                     });
             mIsMutated = true;
@@ -321,6 +323,7 @@ public final class AppSearchSession implements Closeable {
                     request.getProjectionsInternal(),
                     mUserId,
                     new IAppSearchBatchResultCallback.Stub() {
+                        @Override
                         public void onResult(AppSearchBatchResult result) {
                             executor.execute(() -> {
                                 AppSearchBatchResult.Builder<String, GenericDocument>
@@ -359,8 +362,9 @@ public final class AppSearchSession implements Closeable {
                             });
                         }
 
-                        public void onSystemError(ParcelableException exception) {
-                            executor.execute(() -> callback.onSystemError(exception.getCause()));
+                        @Override
+                        public void onSystemError(AppSearchResult result) {
+                            executor.execute(() -> sendSystemErrorToCallback(result, callback));
                         }
                     });
         } catch (RemoteException e) {
@@ -515,12 +519,14 @@ public final class AppSearchSession implements Closeable {
             mService.removeByUri(mPackageName, mDatabaseName, request.getNamespace(),
                     new ArrayList<>(request.getUris()), mUserId,
                     new IAppSearchBatchResultCallback.Stub() {
+                        @Override
                         public void onResult(AppSearchBatchResult result) {
                             executor.execute(() -> callback.onResult(result));
                         }
 
-                        public void onSystemError(ParcelableException exception) {
-                            executor.execute(() -> callback.onSystemError(exception.getCause()));
+                        @Override
+                        public void onSystemError(AppSearchResult result) {
+                            executor.execute(() -> sendSystemErrorToCallback(result, callback));
                         }
                     });
             mIsMutated = true;
@@ -817,4 +823,21 @@ public final class AppSearchSession implements Closeable {
             }
         });
     }
+
+    /**
+     * Calls {@link BatchResultCallback#onSystemError} with a throwable derived from the given
+     * failed {@link AppSearchResult}.
+     *
+     * <p>The {@link AppSearchResult} generally comes from
+     * {@link IAppSearchBatchResultCallback#onSystemError}.
+     *
+     * <p>This method should be called from the callback executor thread.
+     */
+    private void sendSystemErrorToCallback(
+            @NonNull AppSearchResult<?> failedResult, @NonNull BatchResultCallback<?, ?> callback) {
+        Preconditions.checkArgument(!failedResult.isSuccess());
+        Throwable throwable = new AppSearchException(
+                failedResult.getResultCode(), failedResult.getErrorMessage());
+        callback.onSystemError(throwable);
+    }
 }

apex/appsearch/framework/java/android/app/appsearch/BatchResultCallback.java

@@ -36,13 +36,23 @@ public interface BatchResultCallback<KeyType, ValueType> {
     void onResult(@NonNull AppSearchBatchResult<KeyType, ValueType> result);
 
     /**
-     * Called when a system error occurred.
+     * Called when a system error occurs.
      *
-     * @param throwable The cause throwable.
+     * <p>This method is only called the infrastructure is fundamentally broken or unavailable, such
+     * that none of the requests could be started. For example, it will be called if the AppSearch
+     * service unexpectedly fails to initialize and can't be recovered by any means, or if
+     * communicating to the server over Binder fails (e.g. system service crashed or device is
+     * rebooting).
+     *
+     * <p>The error is not expected to be recoverable and there is no specific recommended action
+     * other than displaying a permanent message to the user.
+     *
+     * <p>Normal errors that are caused by invalid inputs or recoverable/retriable situations
+     * are reported associated with the input that caused them via the {@link #onResult} method.
+     *
+     * @param throwable an exception describing the system error
      */
     default void onSystemError(@Nullable Throwable throwable) {
-        if (throwable != null) {
-            throw new RuntimeException(throwable);
-        }
+        throw new RuntimeException("Unrecoverable system error", throwable);
     }
 }

apex/appsearch/framework/java/android/app/appsearch/IAppSearchBatchResultCallback.aidl

@@ -16,10 +16,10 @@
 package android.app.appsearch;
 
 import android.app.appsearch.AppSearchBatchResult;
-import android.os.ParcelableException;
+import android.app.appsearch.AppSearchResult;
 
 /** {@hide} */
 oneway interface IAppSearchBatchResultCallback {
     void onResult(in AppSearchBatchResult result);
-    void onSystemError(in ParcelableException exception);
-}
+    void onSystemError(in AppSearchResult result);
+}

apex/appsearch/framework/java/android/app/appsearch/IAppSearchResultCallback.aidl

@@ -16,9 +16,8 @@
 package android.app.appsearch;
 
 import android.app.appsearch.AppSearchResult;
-import android.os.ParcelableException;
 
 /** {@hide} */
 oneway interface IAppSearchResultCallback {
     void onResult(in AppSearchResult result);
-}
+}

apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java

@@ -44,7 +44,6 @@ import android.content.pm.PackageManagerInternal;
 import android.os.Binder;
 import android.os.Bundle;
 import android.os.ParcelFileDescriptor;
-import android.os.ParcelableException;
 import android.os.RemoteException;
 import android.os.SystemClock;
 import android.os.UserHandle;
@@ -837,13 +836,12 @@ public class AppSearchManagerService extends SystemService {
         /**
          * Invokes the {@link IAppSearchBatchResultCallback} with an unexpected internal throwable.
          *
-         * <p>The throwable is converted to {@link ParcelableException}.
+         * <p>The throwable is converted to {@link AppSearchResult}.
          */
         private void invokeCallbackOnError(
-                IAppSearchBatchResultCallback callback, Throwable throwable) {
+                @NonNull IAppSearchBatchResultCallback callback, @NonNull Throwable throwable) {
             try {
-                //TODO(b/175067650) verify ParcelableException could propagate throwable correctly.
-                callback.onSystemError(new ParcelableException(throwable));
+                callback.onSystemError(throwableToFailedResult(throwable));
             } catch (RemoteException e) {
                 Log.e(TAG, "Unable to send error to the callback", e);
             }

core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java

@@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat;
 
 import static org.testng.Assert.expectThrows;
 
+import android.app.appsearch.exceptions.AppSearchException;
 import android.content.Context;
 
 import androidx.test.core.app.ApplicationProvider;
@@ -89,8 +90,12 @@ public class AppSearchSessionUnitTest {
                 });
 
         // Verify the NullPointException has been thrown.
-        ExecutionException executionException = expectThrows(ExecutionException.class,
-                putDocumentsFuture::get);
-        assertThat(executionException.getCause()).isInstanceOf(NullPointerException.class);
+        ExecutionException executionException =
+                expectThrows(ExecutionException.class, putDocumentsFuture::get);
+        assertThat(executionException.getCause()).isInstanceOf(AppSearchException.class);
+        AppSearchException appSearchException = (AppSearchException) executionException.getCause();
+        assertThat(appSearchException.getResultCode())
+                .isEqualTo(AppSearchResult.RESULT_INTERNAL_ERROR);
+        assertThat(appSearchException.getMessage()).startsWith("NullPointerException");
     }
 }