diff options
6 files changed, 60 insertions, 25 deletions
diff --git a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java index ac91bdb2dce7..c85c4c3a8a7f 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java +++ b/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); + } } diff --git a/apex/appsearch/framework/java/android/app/appsearch/BatchResultCallback.java b/apex/appsearch/framework/java/android/app/appsearch/BatchResultCallback.java index 49049b641839..28f8a7a78f4a 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/BatchResultCallback.java +++ b/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); } } diff --git a/apex/appsearch/framework/java/android/app/appsearch/IAppSearchBatchResultCallback.aidl b/apex/appsearch/framework/java/android/app/appsearch/IAppSearchBatchResultCallback.aidl index b1bbd18b98e2..64b331ea374c 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/IAppSearchBatchResultCallback.aidl +++ b/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); -}
\ No newline at end of file + void onSystemError(in AppSearchResult result); +} diff --git a/apex/appsearch/framework/java/android/app/appsearch/IAppSearchResultCallback.aidl b/apex/appsearch/framework/java/android/app/appsearch/IAppSearchResultCallback.aidl index 27729a5ad058..299c9957974e 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/IAppSearchResultCallback.aidl +++ b/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); -}
\ No newline at end of file +} diff --git a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java index c3f197853872..4ae16aa18305 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java +++ b/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); } diff --git a/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java b/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java index 56c685afa026..3d18337ab257 100644 --- a/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java +++ b/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"); } } |