Show better error messages when the ADB cannot be initialized

Issue: #320
mlopatkin · Dec 25, 2023 · 37b61fc · 37b61fc
1 parent ba919f8
commit 37b61fc
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 20 deletions.
diff --git a/device/src/main/java/name/mlopatkin/andlogview/device/AdbException.java b/device/src/main/java/name/mlopatkin/andlogview/device/AdbException.java
@@ -15,11 +15,9 @@
  */
 package name.mlopatkin.andlogview.device;
 
-public class AdbException extends Exception {
-    public AdbException() {
-        super();
-    }
+import java.util.Objects;
 
+public class AdbException extends Exception {
     public AdbException(String message, Throwable cause) {
         super(message, cause);
     }
@@ -28,7 +26,8 @@ public AdbException(String message) {
         super(message);
     }
 
-    public AdbException(Throwable cause) {
-        super(cause);
+    @Override
+    public String getMessage() {
+        return Objects.requireNonNull(super.getMessage());
     }
 }
diff --git a/device/src/main/java/name/mlopatkin/andlogview/device/AdbLocation.java b/device/src/main/java/name/mlopatkin/andlogview/device/AdbLocation.java
@@ -20,6 +20,13 @@
 import java.util.Optional;
 
 public interface AdbLocation {
+    /**
+     * Returns the current path to the ADB executable as a string, even if it is invalid.
+     *
+     * @return the potentially invalid path to the executable
+     */
+    String getExecutableString();
+
     /**
      * Returns the absolute path to the ADB executable if it is valid. Otherwise, returns an empty Optional.
      *

diff --git a/device/src/main/java/name/mlopatkin/andlogview/device/AdbServerImpl.java b/device/src/main/java/name/mlopatkin/andlogview/device/AdbServerImpl.java
@@ -22,11 +22,13 @@
 
 import com.android.ddmlib.AndroidDebugBridge;
 import com.android.ddmlib.IDevice;
+import com.google.common.base.MoreObjects;
 
 import org.apache.log4j.Logger;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.io.File;
+import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -54,9 +56,33 @@ public AdbDeviceList getDeviceList(Executor listenerExecutor) {
 
     private static AndroidDebugBridge createBridge(AdbLocation adbLocation) throws AdbException {
         logger.info("Starting ADB server");
-        File adbExecutable = adbLocation.getExecutable().orElseThrow(() -> new AdbException("ADB location is invalid"));
+        File adbExecutable = adbLocation.getExecutable()
+                .orElseThrow(
+                        () -> new AdbException("ADB location '" + adbLocation.getExecutableString() + "' is invalid"));
         logger.debug("ADB server executable: " + adbExecutable.getAbsolutePath());
-        @Nullable AndroidDebugBridge bridge = AndroidDebugBridge.createBridge(adbExecutable.getAbsolutePath(), false);
+        final @Nullable AndroidDebugBridge bridge;
+        try {
+            bridge = AndroidDebugBridge.createBridge(adbExecutable.getAbsolutePath(), false);
+        } catch (IllegalArgumentException e) {
+            // Error handling in DDMLIB is a mess.
+            if (e.getCause() instanceof IOException ioException) {
+                throw new AdbException(
+                        String.format(
+                                "Cannot start ADB at '%s':\n%s",
+                                adbExecutable.getAbsolutePath(),
+                                MoreObjects.firstNonNull(ioException.getMessage(), "I/O error")),
+                        ioException
+                );
+            }
+            throw new AdbException(
+                    String.format(
+                            "Failed to initialize ADB at '%s':\n%s",
+                            adbExecutable.getAbsolutePath(),
+                            MoreObjects.firstNonNull(e.getMessage(), "unknown error, see logs")
+                    ),
+                    e
+            );
+        }
         if (bridge == null) {
             throw new AdbException("Failed to create the bridge at " + adbExecutable);
         }

diff --git a/src/name/mlopatkin/andlogview/preferences/AdbConfigurationPref.java b/src/name/mlopatkin/andlogview/preferences/AdbConfigurationPref.java
@@ -143,6 +143,11 @@ private void setRawAdbLocation(String rawAdbLocation, @Nullable File resolvedExe
         }
     }
 
+    @Override
+    public String getExecutableString() {
+        return getAdbLocation();
+    }
+
     @Override
     public Optional<File> getExecutable() {
         return Optional.ofNullable(resolvedExecutable);

diff --git a/src/name/mlopatkin/andlogview/ui/device/AdbNotAvailableDialog.java b/src/name/mlopatkin/andlogview/ui/device/AdbNotAvailableDialog.java
@@ -26,10 +26,10 @@ public class AdbNotAvailableDialog {
 
     private AdbNotAvailableDialog() {}
 
-    public static void show(DialogFactory dialogFactory, Runnable setUpAction) {
+    public static void show(DialogFactory dialogFactory, String failureMessage, Runnable setUpAction) {
         int result = JOptionPane.showOptionDialog(
                 dialogFactory.getOwner(),
-                "The ADB executable cannot be found",
+                failureMessage,
                 "Error",
                 JOptionPane.YES_NO_OPTION,
                 JOptionPane.ERROR_MESSAGE,

diff --git a/src/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenter.java b/src/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenter.java
@@ -20,7 +20,9 @@
 import static name.mlopatkin.andlogview.utils.MyFutures.ignoreCancellations;
 
 import name.mlopatkin.andlogview.AppExecutors;
+import name.mlopatkin.andlogview.base.MyThrowables;
 import name.mlopatkin.andlogview.device.AdbDeviceList;
+import name.mlopatkin.andlogview.device.AdbException;
 import name.mlopatkin.andlogview.liblogcat.ddmlib.DeviceDisconnectedHandler;
 import name.mlopatkin.andlogview.ui.mainframe.MainFrameScoped;
 import name.mlopatkin.andlogview.utils.Cancellable;
@@ -58,7 +60,7 @@ public interface View {
         /**
          * Show the user that ADB services failed to load. This is only called once per presenter lifetime.
          */
-        void showAdbLoadingError();
+        void showAdbLoadingError(String failureReason);
     }
 
     private final AdbServicesBridge bridge;
@@ -146,12 +148,16 @@ private void hideProgressWithToken(Object token) {
     }
 
     private Consumer<Throwable> adbErrorHandler() {
-        return ignored -> {
+        return failure -> {
             if (!hasShownErrorMessage) {
                 hasShownErrorMessage = true;
                 // showAdbLoadingError blocks and opens a nested message pump. It is important to set up the flag before
                 // showing the dialog to prevent re-entrance and double dialog.
-                view.showAdbLoadingError();
+                if (MyThrowables.unwrapUninteresting(failure) instanceof AdbException adbException) {
+                    view.showAdbLoadingError(adbException.getMessage());
+                } else {
+                    view.showAdbLoadingError("Failed to initialize ADB");
+                }
             }
         };
     }

diff --git a/src/name/mlopatkin/andlogview/ui/mainframe/ErrorDialogs.java b/src/name/mlopatkin/andlogview/ui/mainframe/ErrorDialogs.java
@@ -40,8 +40,9 @@ public class ErrorDialogs {
     /**
      * Shows the "The ADB executable was not found" dialog.
      */
-    public void showAdbNotFoundError() {
-        AdbNotAvailableDialog.show(dialogFactory, () -> configurationDialogPresenter.get().openDialog());
+    public void showAdbFailedToStartError(String failureMessage) {
+        AdbNotAvailableDialog.show(dialogFactory, failureMessage,
+                () -> configurationDialogPresenter.get().openDialog());
     }
 
     /**

diff --git a/src/name/mlopatkin/andlogview/ui/mainframe/device/MainFrameAdbInitView.java b/src/name/mlopatkin/andlogview/ui/mainframe/device/MainFrameAdbInitView.java
@@ -49,7 +49,7 @@ public void hideAdbLoadingProgress() {
     }
 
     @Override
-    public void showAdbLoadingError() {
-        errorDialogs.showAdbNotFoundError();
+    public void showAdbLoadingError(String failureReason) {
+        errorDialogs.showAdbFailedToStartError(failureReason);
     }
 }
diff --git a/test/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenterTest.java b/test/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenterTest.java
@@ -276,7 +276,7 @@ void multipleRequestsCauseNoAdditionalErrorsToShow() {
 
         presenter.withAdbDeviceList();
 
-        verify(view, never()).showAdbLoadingError();
+        verify(view, never()).showAdbLoadingError(any());
     }
 
     @ParameterizedTest
@@ -381,11 +381,11 @@ private void thenProgressIsHidden() {
     }
 
     private void thenErrorIsShown() {
-        verify(view).showAdbLoadingError();
+        verify(view).showAdbLoadingError(any());
     }
 
     private void thenNoErrorIsShown() {
-        verify(view, never()).showAdbLoadingError();
+        verify(view, never()).showAdbLoadingError(any());
     }
 
     @FunctionalInterface