Fix exception handling

HMCLCore/src/main/java/org/jackhuang/hmcl/task/TaskExecutor.java

@@ -58,7 +58,7 @@ public final class TaskExecutor {
                     return success;
                 })
                 .exceptionally(e -> {
-                    Lang.handleUncaughtException(e instanceof UncheckedThrowable ? e.getCause() : e);
+                    Lang.handleUncaughtException(resolveException(e));
                     return false;
                 });
         return this;
@@ -101,11 +101,15 @@ public final class TaskExecutor {
                 })
                 .thenApplyAsync(unused -> (Exception) null)
                 .exceptionally(throwable -> {
-                    if (throwable instanceof Exception) {
+                    if (!(throwable instanceof CompletionException))
-                        return (Exception) throwable;
+                        throw new AssertionError();
+
+                    Throwable resolved = resolveException(throwable);
+                    if (resolved instanceof Exception) {
+                        return (Exception) resolved;
                     } else {
                         // If an error occurred, we just rethrow it.
-                        throw new UncheckedThrowable(throwable);
+                        throw (CompletionException) throwable;
                     }
                 });
     }
@@ -117,20 +121,15 @@ public final class TaskExecutor {
     private static void scheduleTo(Scheduler scheduler, ExceptionalRunnable<?> runnable, Runnable finalize) {
         try {
             scheduler.schedule(runnable).get();
-        } catch (ExecutionException e) {
+        } catch (ExecutionException | InterruptedException e) {
-            if (e.getCause() instanceof Exception)
+            throw new CompletionException(e);
-                rethrow(e.getCause());
-            else
-                throw new UncheckedException(e);
-        } catch (InterruptedException e) {
-            throw new UncheckedException(e);
         } finally {
             if (finalize != null)
                 finalize.run();
         }
     }
 
-    private CompletableFuture<Object> executeTask(Task<?> task) {
+    private CompletableFuture<?> executeTask(Task<?> task) {
         return CompletableFuture.completedFuture(null).thenComposeAsync(unused -> {
             task.setState(Task.TaskState.READY);
 
@@ -188,33 +187,36 @@ public final class TaskExecutor {
             task.setState(Task.TaskState.SUCCEEDED);
 
             return null;
-        }).exceptionally(throwable -> {
+        }).thenApplyAsync(unused -> null).exceptionally(throwable -> {
-            if (!(throwable instanceof Exception))
+            if (!(throwable instanceof CompletionException))
-                throw new UncheckedThrowable(throwable);
+                throw new AssertionError();
-            Exception e = throwable instanceof UncheckedException ? (Exception) throwable.getCause() : (Exception) throwable;
+
-            if (e instanceof InterruptedException) {
+            Throwable resolved = resolveException(throwable);
-                task.setException(e);
+            if (resolved instanceof Exception) {
-                if (task.getSignificance().shouldLog()) {
+                Exception e = (Exception) resolved;
-                    Logging.LOG.log(Level.FINE, "Task aborted: " + task.getName());
+                if (e instanceof InterruptedException) {
-                }
-                task.onDone().fireEvent(new TaskEvent(this, task, true));
-                taskListeners.forEach(it -> it.onFailed(task, e));
-            } else if (e instanceof CancellationException || e instanceof RejectedExecutionException) {
-                if (task.getException() == null)
                     task.setException(e);
-            } else {
+                    if (task.getSignificance().shouldLog()) {
-                task.setException(e);
+                        Logging.LOG.log(Level.FINE, "Task aborted: " + task.getName());
-                exception = e;
+                    }
-                if (task.getSignificance().shouldLog()) {
+                    task.onDone().fireEvent(new TaskEvent(this, task, true));
-                    Logging.LOG.log(Level.FINE, "Task failed: " + task.getName(), e);
+                    taskListeners.forEach(it -> it.onFailed(task, e));
+                } else if (e instanceof CancellationException || e instanceof RejectedExecutionException) {
+                    task.setException(e);
+                } else {
+                    task.setException(e);
+                    exception = e;
+                    if (task.getSignificance().shouldLog()) {
+                        Logging.LOG.log(Level.FINE, "Task failed: " + task.getName(), e);
+                    }
+                    task.onDone().fireEvent(new TaskEvent(this, task, true));
+                    taskListeners.forEach(it -> it.onFailed(task, e));
                 }
-                task.onDone().fireEvent(new TaskEvent(this, task, true));
+
-                taskListeners.forEach(it -> it.onFailed(task, e));
+                task.setState(Task.TaskState.FAILED);
             }
 
-            task.setState(Task.TaskState.FAILED);
+            throw (CompletionException) throwable; // rethrow error
-
-            throw new UncheckedException(e);
         });
     }
 
@@ -222,27 +224,20 @@ public final class TaskExecutor {
         return totTask.get();
     }
 
-    private static class UncheckedException extends RuntimeException {
+    private static Throwable resolveException(Throwable e) {
-
+        if (e instanceof ExecutionException || e instanceof CompletionException)
-        UncheckedException(Exception exception) {
+            return resolveException(e.getCause());
-            super(exception);
+        else
-        }
+            return e;
-    }
-
-    private static class UncheckedThrowable extends RuntimeException {
-
-        UncheckedThrowable(Throwable throwable) {
-            super(throwable);
-        }
     }
 
     private static void rethrow(Throwable e) {
-        if (e instanceof RuntimeException) { // including UncheckedException and UncheckedThrowable
+        if (e instanceof ExecutionException || e instanceof CompletionException) { // including UncheckedException and UncheckedThrowable
+            rethrow(e.getCause());
+        } else if (e instanceof RuntimeException) {
             throw (RuntimeException) e;
-        } else if (e instanceof Exception) {
-            throw new UncheckedException((Exception) e);
         } else {
-            throw new UncheckedThrowable(e);
+            throw new CompletionException(e);
         }
     }
 }

HMCLCore/src/test/java/org/jackhuang/hmcl/util/TaskTest.java

@@ -4,15 +4,48 @@ import org.jackhuang.hmcl.task.Task;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+
 public class TaskTest {
+    /**
+     * TaskExecutor will not catch error and will be thrown to global handler.
+     */
+    @Test
+    public void expectErrorUncaught() {
+        AtomicReference<Throwable> throwable = new AtomicReference<>();
+        Thread.setDefaultUncaughtExceptionHandler((t, e) -> throwable.set(e));
+        Assert.assertFalse(Task.composeAsync(() -> Task.allOf(
+                Task.allOf(Task.runAsync(() -> {
+                    throw new Error();
+                }))
+        )).whenComplete(Assert::assertNull).test());
+
+        Assert.assertTrue(throwable.get() instanceof Error);
+    }
+
+    /**
+     *
+     */
     @Test
     public void testWhenComplete() {
-        Task.composeAsync(() -> Task.allOf(
+        Assert.assertFalse(Task.supplyAsync(() -> {
-                Task.allOf(Task.runAsync(() -> {
+            throw new IllegalStateException();
-                    throw new Exception();
+        }).whenComplete(exception -> {
-                }))
+            Assert.assertTrue(exception instanceof IllegalStateException);
-        )).whenComplete((exception -> {
+        }).test());
-            Assert.assertFalse(exception == null);
-        })).test();
     }
-}
+
+    @Test
+    public void testWithCompose() {
+        AtomicBoolean bool = new AtomicBoolean();
+        boolean success = Task.supplyAsync(() -> {
+            throw new IllegalStateException();
+        }).withRun(() -> {
+            bool.set(true);
+        }).test();
+
+        Assert.assertTrue(success);
+        Assert.assertTrue(bool.get());
+    }
+}