From e408fdcc084ce38149e1156f828cb2613e55f34a Mon Sep 17 00:00:00 2001 From: agnostic-apollo Date: Mon, 28 Jun 2021 09:19:20 +0500 Subject: [PATCH] Show crash notification when bootstrap installation or setup storage failures Sometimes users report that bootstrap installation failed on their devices but provide no details. Since they don't check logcat for the exception or exception is one time only, we can't know what happened. Although, reasons are likely root ownership files. The notification will show the full stacktrace including suppressed ones for why failure occurred and hopefully be easier to find the problems and we can get reports too. --- .../java/com/termux/app/TermuxActivity.java | 2 +- .../java/com/termux/app/TermuxInstaller.java | 92 +++++++++++-------- .../java/com/termux/app/utils/CrashUtils.java | 76 ++++++++++----- 3 files changed, 109 insertions(+), 61 deletions(-) diff --git a/app/src/main/java/com/termux/app/TermuxActivity.java b/app/src/main/java/com/termux/app/TermuxActivity.java index a3885fe5..bd54677e 100644 --- a/app/src/main/java/com/termux/app/TermuxActivity.java +++ b/app/src/main/java/com/termux/app/TermuxActivity.java @@ -178,7 +178,7 @@ public final class TermuxActivity extends Activity implements ServiceConnection // Check if a crash happened on last run of the app and show a // notification with the crash details if it did - CrashUtils.notifyCrash(this, LOG_TAG); + CrashUtils.notifyAppCrashOnLastRun(this, LOG_TAG); // Load termux shared properties mProperties = new TermuxAppSharedProperties(this); diff --git a/app/src/main/java/com/termux/app/TermuxInstaller.java b/app/src/main/java/com/termux/app/TermuxInstaller.java index 6b4782eb..83ce3ab9 100644 --- a/app/src/main/java/com/termux/app/TermuxInstaller.java +++ b/app/src/main/java/com/termux/app/TermuxInstaller.java @@ -11,6 +11,7 @@ import android.util.Pair; import android.view.WindowManager; import com.termux.R; +import com.termux.app.utils.CrashUtils; import com.termux.shared.file.FileUtils; import com.termux.shared.interact.DialogUtils; import com.termux.shared.logger.Logger; @@ -70,14 +71,14 @@ final class TermuxInstaller { // If prefix directory exists, even if its a symlink to a valid directory and symlink is not broken/dangling if (FileUtils.directoryFileExists(PREFIX_FILE_PATH, true)) { - File[] PREFIX_FILE_LIST = PREFIX_FILE.listFiles(); + File[] PREFIX_FILE_LIST = PREFIX_FILE.listFiles(); // If prefix directory is empty or only contains the tmp directory - if(PREFIX_FILE_LIST == null || PREFIX_FILE_LIST.length == 0 || (PREFIX_FILE_LIST.length == 1 && TermuxConstants.TERMUX_TMP_PREFIX_DIR_PATH.equals(PREFIX_FILE_LIST[0].getAbsolutePath()))) { - Logger.logInfo(LOG_TAG, "The prefix directory \"" + PREFIX_FILE_PATH + "\" exists but is empty or only contains the tmp directory."); - } else { - whenDone.run(); - return; - } + if(PREFIX_FILE_LIST == null || PREFIX_FILE_LIST.length == 0 || (PREFIX_FILE_LIST.length == 1 && TermuxConstants.TERMUX_TMP_PREFIX_DIR_PATH.equals(PREFIX_FILE_LIST[0].getAbsolutePath()))) { + Logger.logInfo(LOG_TAG, "The prefix directory \"" + PREFIX_FILE_PATH + "\" exists but is empty or only contains the tmp directory."); + } else { + whenDone.run(); + return; + } } else if (FileUtils.fileExists(PREFIX_FILE_PATH, false)) { Logger.logInfo(LOG_TAG, "The prefix directory \"" + PREFIX_FILE_PATH + "\" does not exist but another file exists at its destination."); } @@ -97,13 +98,15 @@ final class TermuxInstaller { // Delete prefix staging directory or any file at its destination error = FileUtils.deleteFile("prefix staging directory", STAGING_PREFIX_PATH, true); if (error != null) { - throw new RuntimeException(error.toString()); + showBootstrapErrorDialog(activity, PREFIX_FILE_PATH, whenDone, Error.getErrorMarkdownString(error)); + return; } // Delete prefix directory or any file at its destination error = FileUtils.deleteFile("prefix directory", PREFIX_FILE_PATH, true); if (error != null) { - throw new RuntimeException(error.toString()); + showBootstrapErrorDialog(activity, PREFIX_FILE_PATH, whenDone, Error.getErrorMarkdownString(error)); + return; } Logger.logInfo(LOG_TAG, "Extracting bootstrap zip to prefix staging directory \"" + STAGING_PREFIX_PATH + "\"."); @@ -126,14 +129,22 @@ final class TermuxInstaller { String newPath = STAGING_PREFIX_PATH + "/" + parts[1]; symlinks.add(Pair.create(oldPath, newPath)); - ensureDirectoryExists(new File(newPath).getParentFile()); + error = ensureDirectoryExists(new File(newPath).getParentFile()); + if (error != null) { + showBootstrapErrorDialog(activity, PREFIX_FILE_PATH, whenDone, Error.getErrorMarkdownString(error)); + return; + } } } else { String zipEntryName = zipEntry.getName(); File targetFile = new File(STAGING_PREFIX_PATH, zipEntryName); boolean isDirectory = zipEntry.isDirectory(); - ensureDirectoryExists(isDirectory ? targetFile : targetFile.getParentFile()); + error = ensureDirectoryExists(isDirectory ? targetFile : targetFile.getParentFile()); + if (error != null) { + showBootstrapErrorDialog(activity, PREFIX_FILE_PATH, whenDone, Error.getErrorMarkdownString(error)); + return; + } if (!isDirectory) { try (FileOutputStream outStream = new FileOutputStream(targetFile)) { @@ -164,23 +175,10 @@ final class TermuxInstaller { Logger.logInfo(LOG_TAG, "Bootstrap packages installed successfully."); activity.runOnUiThread(whenDone); + } catch (final Exception e) { - Logger.logStackTraceWithMessage(LOG_TAG, "Bootstrap error", e); - activity.runOnUiThread(() -> { - try { - new AlertDialog.Builder(activity).setTitle(R.string.bootstrap_error_title).setMessage(R.string.bootstrap_error_body) - .setNegativeButton(R.string.bootstrap_error_abort, (dialog, which) -> { - dialog.dismiss(); - activity.finish(); - }).setPositiveButton(R.string.bootstrap_error_try_again, (dialog, which) -> { - dialog.dismiss(); - FileUtils.deleteFile("prefix directory", PREFIX_FILE_PATH, true); - TermuxInstaller.setupBootstrapIfNeeded(activity, whenDone); - }).show(); - } catch (WindowManager.BadTokenException e1) { - // Activity already dismissed - ignore. - } - }); + showBootstrapErrorDialog(activity, PREFIX_FILE_PATH, whenDone, Logger.getStackTracesMarkdownString(null, Logger.getStackTracesStringArray(e))); + } finally { activity.runOnUiThread(() -> { try { @@ -194,6 +192,30 @@ final class TermuxInstaller { }.start(); } + public static void showBootstrapErrorDialog(Activity activity, String PREFIX_FILE_PATH, Runnable whenDone, String message) { + Logger.logErrorExtended(LOG_TAG, "Bootstrap Error:\n" + message); + + // Send a notification with the exception so that the user knows why bootstrap setup failed + CrashUtils.sendCrashReportNotification(activity, LOG_TAG, "## Bootstrap Error\n\n" + message, true); + + activity.runOnUiThread(() -> { + try { + new AlertDialog.Builder(activity).setTitle(R.string.bootstrap_error_title).setMessage(R.string.bootstrap_error_body) + .setNegativeButton(R.string.bootstrap_error_abort, (dialog, which) -> { + dialog.dismiss(); + activity.finish(); + }) + .setPositiveButton(R.string.bootstrap_error_try_again, (dialog, which) -> { + dialog.dismiss(); + FileUtils.deleteFile("prefix directory", PREFIX_FILE_PATH, true); + TermuxInstaller.setupBootstrapIfNeeded(activity, whenDone); + }).show(); + } catch (WindowManager.BadTokenException e1) { + // Activity already dismissed - ignore. + } + }); + } + static void setupStorageSymlinks(final Context context) { final String LOG_TAG = "termux-storage"; @@ -208,7 +230,8 @@ final class TermuxInstaller { error = FileUtils.clearDirectory("~/storage", storageDir.getAbsolutePath()); if (error != null) { Logger.logErrorAndShowToast(context, LOG_TAG, error.getMessage()); - Logger.logErrorExtended(LOG_TAG, error.toString()); + Logger.logErrorExtended(LOG_TAG, "Setup Storage Error\n" + error.toString()); + CrashUtils.sendCrashReportNotification(context, LOG_TAG, "## Setup Storage Error\n\n" + Error.getErrorMarkdownString(error), true); return; } @@ -245,19 +268,16 @@ final class TermuxInstaller { Logger.logInfo(LOG_TAG, "Storage symlinks created successfully."); } catch (Exception e) { - Logger.logStackTraceWithMessage(LOG_TAG, "Error setting up link", e); + Logger.logErrorAndShowToast(context, LOG_TAG, e.getMessage()); + Logger.logStackTraceWithMessage(LOG_TAG, "Setup Storage Error: Error setting up link", e); + CrashUtils.sendCrashReportNotification(context, LOG_TAG, "## Setup Storage Error\n\n" + Logger.getStackTracesMarkdownString(null, Logger.getStackTracesStringArray(e)), true); } } }.start(); } - private static void ensureDirectoryExists(File directory) { - Error error; - - error = FileUtils.createDirectoryFile(directory.getAbsolutePath()); - if (error != null) { - throw new RuntimeException(error.toString()); - } + private static Error ensureDirectoryExists(File directory) { + return FileUtils.createDirectoryFile(directory.getAbsolutePath()); } public static byte[] loadZipBytes() { diff --git a/app/src/main/java/com/termux/app/utils/CrashUtils.java b/app/src/main/java/com/termux/app/utils/CrashUtils.java index 63cd0356..3a59d9c1 100644 --- a/app/src/main/java/com/termux/app/utils/CrashUtils.java +++ b/app/src/main/java/com/termux/app/utils/CrashUtils.java @@ -30,8 +30,8 @@ public class CrashUtils { private static final String LOG_TAG = "CrashUtils"; /** - * Notify the user of a previous app crash by reading the crash info from the crash log file at - * {@link TermuxConstants#TERMUX_CRASH_LOG_FILE_PATH}. The crash log file would have been + * Notify the user of an app crash at last run by reading the crash info from the crash log file + * at {@link TermuxConstants#TERMUX_CRASH_LOG_FILE_PATH}. The crash log file would have been * created by {@link com.termux.shared.crash.CrashHandler}. * * If the crash log file exists and is not empty and @@ -44,10 +44,9 @@ public class CrashUtils { * @param context The {@link Context} for operations. * @param logTagParam The log tag to use for logging. */ - public static void notifyCrash(final Context context, final String logTagParam) { + public static void notifyAppCrashOnLastRun(final Context context, final String logTagParam) { if (context == null) return; - TermuxAppSharedPreferences preferences = TermuxAppSharedPreferences.build(context); if (preferences == null) return; @@ -84,31 +83,60 @@ public class CrashUtils { if (reportString.isEmpty()) return; - // Send a notification to show the crash log which when clicked will open the {@link ReportActivity} - // to show the details of the crash - String title = TermuxConstants.TERMUX_APP_NAME + " Crash Report"; + Logger.logDebug(logTag, "A crash log file found at \"" + TermuxConstants.TERMUX_CRASH_LOG_FILE_PATH + "\"."); - Logger.logDebug(logTag, "The crash log file at \"" + TermuxConstants.TERMUX_CRASH_LOG_FILE_PATH + "\" found. Sending \"" + title + "\" notification."); - - Intent notificationIntent = ReportActivity.newInstance(context, new ReportInfo(UserAction.CRASH_REPORT.getName(), logTag, title, null, reportString, "\n\n" + TermuxUtils.getReportIssueMarkdownString(context), true)); - PendingIntent pendingIntent = PendingIntent.getActivity(context, 0, notificationIntent, PendingIntent.FLAG_UPDATE_CURRENT); - - // Setup the notification channel if not already set up - setupCrashReportsNotificationChannel(context); - - // Build the notification - Notification.Builder builder = getCrashReportsNotificationBuilder(context, title, null, null, pendingIntent, NotificationUtils.NOTIFICATION_MODE_VIBRATE); - if (builder == null) return; - - // Send the notification - int nextNotificationId = NotificationUtils.getNextNotificationId(context); - NotificationManager notificationManager = NotificationUtils.getNotificationManager(context); - if (notificationManager != null) - notificationManager.notify(nextNotificationId, builder.build()); + sendCrashReportNotification(context, logTag, reportString, false); } }.start(); } + /** + * Send a crash report notification for {@link TermuxConstants#TERMUX_CRASH_REPORTS_NOTIFICATION_CHANNEL_ID} + * and {@link TermuxConstants#TERMUX_CRASH_REPORTS_NOTIFICATION_CHANNEL_NAME}. + * + * @param context The {@link Context} for operations. + * @param logTag The log tag to use for logging. + * @param reportString The text for the crash report. + * @param forceNotification If set to {@code true}, then a notification will be shown + * regardless of if pending intent is {@code null} or + * {@link TermuxPreferenceConstants.TERMUX_APP#KEY_CRASH_REPORT_NOTIFICATIONS_ENABLED} + * is {@code false}. + */ + public static void sendCrashReportNotification(final Context context, String logTag, String reportString, boolean forceNotification) { + if (context == null) return; + + TermuxAppSharedPreferences preferences = TermuxAppSharedPreferences.build(context); + if (preferences == null) return; + + // If user has disabled notifications for crashes + if (!preferences.areCrashReportNotificationsEnabled() && !forceNotification) + return; + + logTag = DataUtils.getDefaultIfNull(logTag, LOG_TAG); + + // Send a notification to show the crash log which when clicked will open the {@link ReportActivity} + // to show the details of the crash + String title = TermuxConstants.TERMUX_APP_NAME + " Crash Report"; + + Logger.logDebug(logTag, "Sending \"" + title + "\" notification."); + + Intent notificationIntent = ReportActivity.newInstance(context, new ReportInfo(UserAction.CRASH_REPORT.getName(), logTag, title, null, reportString, "\n\n" + TermuxUtils.getReportIssueMarkdownString(context), true)); + PendingIntent pendingIntent = PendingIntent.getActivity(context, 0, notificationIntent, PendingIntent.FLAG_UPDATE_CURRENT); + + // Setup the notification channel if not already set up + setupCrashReportsNotificationChannel(context); + + // Build the notification + Notification.Builder builder = getCrashReportsNotificationBuilder(context, title, null, null, pendingIntent, NotificationUtils.NOTIFICATION_MODE_VIBRATE); + if (builder == null) return; + + // Send the notification + int nextNotificationId = NotificationUtils.getNextNotificationId(context); + NotificationManager notificationManager = NotificationUtils.getNotificationManager(context); + if (notificationManager != null) + notificationManager.notify(nextNotificationId, builder.build()); + } + /** * Get {@link Notification.Builder} for {@link TermuxConstants#TERMUX_CRASH_REPORTS_NOTIFICATION_CHANNEL_ID} * and {@link TermuxConstants#TERMUX_CRASH_REPORTS_NOTIFICATION_CHANNEL_NAME}.