[RESTRICT AUTOMERGE] Gracefull handle car-service and dumpstate errors.
- BugReportService: Connects to car-service when bugreport is requested.
- BugReportService disconnects from car-service right when bugreporting
is finished or failed.
When it's failed it changes the status of bugreport entry in DB.
- if car-service fails, stop BugReportService after some delay
to allow toast messages to show.
Bug: 149311456
Bug: 149228567
Test: on a elk bench
Test: crashed car-service and checked if the app handled it properly
Change-Id: I07f61a2162c15c95ffe68b098ec4f5e1c4b47093
diff --git a/tests/BugReportApp/AndroidManifest.xml b/tests/BugReportApp/AndroidManifest.xml
index 5da91d4..dcccb83 100644
--- a/tests/BugReportApp/AndroidManifest.xml
+++ b/tests/BugReportApp/AndroidManifest.xml
@@ -16,8 +16,8 @@
-->
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.google.android.car.bugreport"
- android:versionCode="12"
- android:versionName="1.7.1">
+ android:versionCode="13"
+ android:versionName="1.7.2">
<uses-permission android:name="android.car.permission.CAR_DRIVING_STATE"/>
<uses-permission android:name="android.permission.INTERNET"/>
diff --git a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportActivity.java b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportActivity.java
index a5aee0f..20fe5f9 100644
--- a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportActivity.java
+++ b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportActivity.java
@@ -137,26 +137,6 @@
}
};
- private final ServiceConnection mCarServiceConnection = new ServiceConnection() {
- @Override
- public void onServiceConnected(ComponentName name, IBinder service) {
- try {
- mDrivingStateManager = (CarDrivingStateManager) mCar.getCarManager(
- Car.CAR_DRIVING_STATE_SERVICE);
- mDrivingStateManager.registerListener(
- BugReportActivity.this::onCarDrivingStateChanged);
- // Call onCarDrivingStateChanged(), because it's not called when Car is connected.
- onCarDrivingStateChanged(mDrivingStateManager.getCurrentCarDrivingState());
- } catch (CarNotConnectedException e) {
- Log.w(TAG, "Failed to get CarDrivingStateManager.", e);
- }
- }
-
- @Override
- public void onServiceDisconnected(ComponentName name) {
- }
- };
-
/**
* Builds an intent that starts {@link BugReportActivity} to add audio message to the existing
* bug report.
@@ -227,6 +207,10 @@
}
private void onCarDrivingStateChanged(CarDrivingStateEvent event) {
+ if (mShowBugReportsButton == null) {
+ Log.w(TAG, "Cannot handle driving state change, UI is not ready");
+ return;
+ }
// When adding audio message to the existing bugreport, do not show "Show Bug Reports"
// button, users either should explicitly Submit or Cancel.
if (mAudioRecordingStarted && !mIsNewBugReport) {
@@ -260,8 +244,8 @@
// We need to minimize system state change when performing SILENT bug report.
mConfig = new Config();
mConfig.start();
- mCar = Car.createCar(this, mCarServiceConnection);
- mCar.connect();
+ mCar = Car.createCar(this, /* handler= */ null,
+ Car.CAR_WAIT_TIMEOUT_DO_NOT_WAIT, this::onCarLifecycleChanged);
mInProgressTitleText = findViewById(R.id.in_progress_title_text);
mProgressBar = findViewById(R.id.progress_bar);
@@ -287,6 +271,27 @@
}
}
+ private void onCarLifecycleChanged(Car car, boolean ready) {
+ if (!ready) {
+ mDrivingStateManager = null;
+ mCar = null;
+ Log.d(TAG, "Car service is not ready, ignoring");
+ // If car service is not ready for this activity, just ignore it - as it's only
+ // used to control UX restrictions.
+ return;
+ }
+ try {
+ mDrivingStateManager = (CarDrivingStateManager) car.getCarManager(
+ Car.CAR_DRIVING_STATE_SERVICE);
+ mDrivingStateManager.registerListener(
+ BugReportActivity.this::onCarDrivingStateChanged);
+ // Call onCarDrivingStateChanged(), because it's not called when Car is connected.
+ onCarDrivingStateChanged(mDrivingStateManager.getCurrentCarDrivingState());
+ } catch (CarNotConnectedException e) {
+ Log.w(TAG, "Failed to get CarDrivingStateManager", e);
+ }
+ }
+
private void showInProgressUi() {
mSubmitBugReportLayout.setVisibility(View.GONE);
mInProgressLayout.setVisibility(View.VISIBLE);
diff --git a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
index f1acd83..2bebd72 100644
--- a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
+++ b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
@@ -15,6 +15,10 @@
*/
package com.google.android.car.bugreport;
+import static android.car.CarBugreportManager.CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED;
+import static android.car.CarBugreportManager.CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_FAILED;
+import static android.car.CarBugreportManager.CarBugreportManagerCallback.CAR_BUGREPORT_SERVICE_NOT_AVAILABLE;
+
import static com.google.android.car.bugreport.PackageUtils.getPackageVersion;
import android.annotation.FloatRange;
@@ -89,6 +93,9 @@
// this, the best option is probably to wait for onDetach events from view tree.
private static final int ACTIVITY_FINISH_DELAY_MILLIS = 1000;
+ /** Stop the service only after some delay, to allow toasts to show on the screen. */
+ private static final int STOP_SERVICE_DELAY_MILLIS = 1000;
+
/**
* Wait a short time before showing "bugreport started" toast message, because the service
* will take a screenshot of the screen.
@@ -124,6 +131,7 @@
/** Binder given to clients. */
private final IBinder mBinder = new ServiceBinder();
+ /** True if {@link BugReportService} is already collecting bugreport, including zipping. */
private final AtomicBoolean mIsCollectingBugReport = new AtomicBoolean(false);
private final AtomicDouble mBugReportProgress = new AtomicDouble(0);
@@ -143,7 +151,7 @@
* finishes. We need to clear it otherwise when bugreport fails, it will show "bugreport start"
* toast, which will confuse users.
*/
- private Handler mHandlerToast;
+ private Handler mHandlerStartedToast;
/** A listener that's notified when bugreport progress changes. */
interface BugReportProgressListener {
@@ -196,16 +204,9 @@
NotificationManager.IMPORTANCE_HIGH));
mSingleThreadExecutor = Executors.newSingleThreadScheduledExecutor();
mHandler = new BugReportHandler();
- mHandlerToast = new Handler();
+ mHandlerStartedToast = new Handler();
mConfig = new Config();
mConfig.start();
- // Synchronously connect to the car service.
- mCar = Car.createCar(this);
- try {
- mBugreportManager = (CarBugreportManager) mCar.getCarManager(Car.CAR_BUGREPORT_SERVICE);
- } catch (CarNotConnectedException | NoClassDefFoundError e) {
- throw new IllegalStateException("Failed to get CarBugreportManager.", e);
- }
}
@Override
@@ -213,7 +214,7 @@
if (DEBUG) {
Log.d(TAG, "Service destroyed");
}
- mCar.disconnect();
+ disconnectFromCarService();
}
@Override
@@ -243,7 +244,7 @@
collectBugReport();
// Show a short lived "bugreport started" toast message after a short delay.
- mHandlerToast.postDelayed(() -> {
+ mHandlerStartedToast.postDelayed(() -> {
Toast.makeText(this,
getText(R.string.toast_bug_report_started), Toast.LENGTH_LONG).show();
}, BUGREPORT_STARTED_TOAST_DELAY_MILLIS);
@@ -252,6 +253,24 @@
return START_NOT_STICKY;
}
+ private void onCarLifecycleChanged(Car car, boolean ready) {
+ // not ready - car service is crashed or is restarting.
+ if (!ready) {
+ mBugreportManager = null;
+ mCar = null;
+
+ // NOTE: dumpstate still might be running, but we can't kill it or reconnect to it
+ // so we ignore it.
+ handleBugReportManagerError(CAR_BUGREPORT_SERVICE_NOT_AVAILABLE);
+ return;
+ }
+ try {
+ mBugreportManager = (CarBugreportManager) car.getCarManager(Car.CAR_BUGREPORT_SERVICE);
+ } catch (CarNotConnectedException | NoClassDefFoundError e) {
+ throw new IllegalStateException("Failed to get CarBugreportManager.", e);
+ }
+ }
+
/** Shows an updated progress notification. */
private void showProgressNotification() {
if (isCollectingBugReport()) {
@@ -306,7 +325,26 @@
mHandler.post(() -> Toast.makeText(this, getText(resId), Toast.LENGTH_LONG).show());
}
+ private void disconnectFromCarService() {
+ if (mCar != null) {
+ mCar.disconnect();
+ mCar = null;
+ }
+ mBugreportManager = null;
+ }
+
+ private void connectToCarServiceSync() {
+ if (mCar == null || !(mCar.isConnected() || mCar.isConnecting())) {
+ mCar = Car.createCar(this, /* handler= */ null,
+ Car.CAR_WAIT_TIMEOUT_WAIT_FOREVER, this::onCarLifecycleChanged);
+ }
+ }
+
private void collectBugReport() {
+ // Connect to the car service before collecting bugreport, because when car service crashes,
+ // BugReportService doesn't automatically reconnect to it.
+ connectToCarServiceSync();
+
if (Build.IS_USERDEBUG || Build.IS_ENG) {
mSingleThreadExecutor.schedule(
this::grabBtSnoopLog, ACTIVITY_FINISH_DELAY_MILLIS, TimeUnit.MILLISECONDS);
@@ -348,6 +386,8 @@
BugStorageUtils.setBugReportStatus(this, mMetaBugReport, Status.STATUS_WRITE_FAILED,
MESSAGE_FAILURE_DUMPSTATE);
showToast(R.string.toast_status_dump_state_failed);
+ disconnectFromCarService();
+ mIsCollectingBugReport.set(false);
}
}
@@ -364,22 +404,10 @@
}
mCallback = new CarBugreportManager.CarBugreportManagerCallback() {
@Override
- public void onError(int errorCode) {
+ public void onError(@CarBugreportErrorCode int errorCode) {
Log.e(TAG, "CarBugreportManager failed: " + errorCode);
- // We let the UI know that bug reporting is finished, because the next step is to
- // zip everything and upload.
- mBugReportProgress.set(MAX_PROGRESS_VALUE);
- sendProgressEventToHandler(MAX_PROGRESS_VALUE);
- showToast(R.string.toast_status_failed);
- BugStorageUtils.setBugReportStatus(
- BugReportService.this, mMetaBugReport,
- Status.STATUS_WRITE_FAILED, "CarBugreportManager failed: " + errorCode);
- mIsCollectingBugReport.set(false);
- mHandler.post(() -> {
- mNotificationManager.cancel(BUGREPORT_IN_PROGRESS_NOTIF_ID);
- stopForeground(true);
- });
- mHandlerToast.removeCallbacksAndMessages(null);
+ disconnectFromCarService();
+ handleBugReportManagerError(errorCode);
}
@Override
@@ -391,14 +419,58 @@
@Override
public void onFinished() {
Log.d(TAG, "CarBugreportManager finished");
+ disconnectFromCarService();
mBugReportProgress.set(MAX_PROGRESS_VALUE);
sendProgressEventToHandler(MAX_PROGRESS_VALUE);
mSingleThreadExecutor.submit(BugReportService.this::zipDirectoryAndUpdateStatus);
}
};
+ if (mBugreportManager == null) {
+ mHandler.post(() -> Toast.makeText(this,
+ "Car service is not ready", Toast.LENGTH_LONG).show());
+ Log.e(TAG, "CarBugReportManager is not ready");
+ return;
+ }
mBugreportManager.requestBugreport(outFd, extraOutFd, mCallback);
}
+ private void handleBugReportManagerError(
+ @CarBugreportManager.CarBugreportManagerCallback.CarBugreportErrorCode int errorCode) {
+ if (mMetaBugReport == null) {
+ Log.w(TAG, "No bugreport is running");
+ mIsCollectingBugReport.set(false);
+ return;
+ }
+ // We let the UI know that bug reporting is finished, because the next step is to
+ // zip everything and upload.
+ mBugReportProgress.set(MAX_PROGRESS_VALUE);
+ sendProgressEventToHandler(MAX_PROGRESS_VALUE);
+ showToast(R.string.toast_status_failed);
+ BugStorageUtils.setBugReportStatus(
+ BugReportService.this, mMetaBugReport,
+ Status.STATUS_WRITE_FAILED, getBugReportFailureStatusMessage(errorCode));
+ mHandler.postDelayed(() -> {
+ mNotificationManager.cancel(BUGREPORT_IN_PROGRESS_NOTIF_ID);
+ stopForeground(true);
+ }, STOP_SERVICE_DELAY_MILLIS);
+ mHandlerStartedToast.removeCallbacksAndMessages(null);
+ mMetaBugReport = null;
+ mIsCollectingBugReport.set(false);
+ }
+
+ private static String getBugReportFailureStatusMessage(
+ @CarBugreportManager.CarBugreportManagerCallback.CarBugreportErrorCode int errorCode) {
+ switch (errorCode) {
+ case CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED:
+ case CAR_BUGREPORT_DUMPSTATE_FAILED:
+ return "Failed to connect to dumpstate. Retry again after a minute.";
+ case CAR_BUGREPORT_SERVICE_NOT_AVAILABLE:
+ return "Car service is not available. Retry again.";
+ default:
+ return "Car service bugreport collection failed: " + errorCode;
+ }
+ }
+
/**
* Shows a clickable bugreport finished notification. When clicked it opens
* {@link BugReportInfoActivity}.
@@ -440,7 +512,6 @@
File bugReportTempDir = FileUtils.createTempDir(this, mMetaBugReport.getTimestamp());
zipDirectoryToOutputStream(bugReportTempDir,
BugStorageUtils.openBugReportFileToWrite(this, mMetaBugReport));
- mIsCollectingBugReport.set(false);
} catch (IOException e) {
Log.e(TAG, "Failed to zip files", e);
BugStorageUtils.setBugReportStatus(this, mMetaBugReport, Status.STATUS_WRITE_FAILED,
@@ -465,7 +536,9 @@
mNotificationManager.cancel(BUGREPORT_IN_PROGRESS_NOTIF_ID);
stopForeground(true);
});
- mHandlerToast.removeCallbacksAndMessages(null);
+ mHandlerStartedToast.removeCallbacksAndMessages(null);
+ mMetaBugReport = null;
+ mIsCollectingBugReport.set(false);
}
private void playNotificationSound() {
