Bug 558642: SafeRunner can return results
SafeRunner has a second run method which expects a
ISafeRunnableWithResult<T>. The run method returns this result.
Also some refactoring of the handleException method has been done (boy
scout rule).
Furthermore test have been improved:
- adapt to lambda
- better test method names to recognize what was wrong
- better text messages in assertions
Change-Id: I3a648490f85101d87119335310c03774371408af
Signed-off-by: Marcus Hoepfner <marcus.hoepfner@sap.com>
diff --git a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java
index 7edf55d..c209ab8 100644
--- a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java
+++ b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java
@@ -13,7 +13,10 @@
*******************************************************************************/
package org.eclipse.equinox.common.tests;
-import org.eclipse.core.runtime.*;
+import org.eclipse.core.runtime.ISafeRunnable;
+import org.eclipse.core.runtime.ISafeRunnableWithResult;
+import org.eclipse.core.runtime.OperationCanceledException;
+import org.eclipse.core.runtime.SafeRunner;
import org.eclipse.core.tests.harness.CoreTest;
/**
@@ -22,89 +25,36 @@
public class SafeRunnerTest extends CoreTest {
/**
- * Ensures that cancelation exceptions are handled
+ * Ensures that cancellation exceptions are handled
*/
- public void testCancel() {
- boolean caught = false;
+ public void testOperationCanceledExceptionAreHandled() {
try {
- SafeRunner.run(new ISafeRunnable() {
- @Override
- public void handleException(Throwable exception) {
- }
-
- @Override
- public void run() throws Exception {
- throw new OperationCanceledException();
- }
+ SafeRunner.run(() -> {
+ throw new OperationCanceledException();
});
} catch (OperationCanceledException e) {
- caught = true;
+ fail("OperationCanceledException unexpectedly caught.", e);
}
- assertFalse("1.0", caught);
}
- /**
- * Tests that SafeRunner catches the expected exception types.
- */
- public void testCaughtExceptionTypes() {
- Throwable[] failures = new Throwable[] {new AssertionError(), new LinkageError(), new RuntimeException()};
- for (int i = 0; i < failures.length; i++) {
- final Throwable[] handled = new Throwable[1];
- final Throwable current = failures[i];
- try {
- SafeRunner.run(new ISafeRunnable() {
- @Override
- public void handleException(Throwable exception) {
- handled[0] = exception;
- }
-
- @Override
- public void run() throws Exception {
- if (current instanceof Exception) {
- throw (Exception) current;
- } else if (current instanceof Error) {
- throw (Error) current;
- }
- }
- });
- } catch (Throwable t) {
- fail("1." + i, t);
- }
- assertEquals("2." + i, current, handled[0]);
- }
-
+ public void testAssertionErrorIsCaught() {
+ assertExceptionHandled(new AssertionError());
}
- /**
- * Tests that SafeRunner re-throws expected exception types.
- */
- public void testThrownExceptionTypes() {
- //thrown exceptions
- final Throwable[] thrown = new Throwable[] {new Error(), new OutOfMemoryError()};
- for (int i = 0; i < thrown.length; i++) {
- boolean caught = false;
- final Throwable current = thrown[i];
- try {
- SafeRunner.run(new ISafeRunnable() {
- @Override
- public void handleException(Throwable exception) {
- }
+ public void testLinkageErrorIsCaught() {
+ assertExceptionHandled(new LinkageError());
+ }
- @Override
- public void run() throws Exception {
- if (current instanceof Exception) {
- throw (Exception) current;
- } else if (current instanceof Error) {
- throw (Error) current;
- }
- }
- });
- } catch (Throwable t) {
- assertEquals("1." + i, current, t);
- caught = true;
- }
- assertTrue("2." + i, caught);
- }
+ public void testRuntimeExceptionIsCaught() {
+ assertExceptionHandled(new RuntimeException());
+ }
+
+ public void testRethrowsError() {
+ assertExceptionRethrown(new Error());
+ }
+
+ public void testRethrowsOutOfMemoryError() {
+ assertExceptionRethrown(new OutOfMemoryError());
}
public void testNull() {
@@ -112,7 +62,7 @@
SafeRunner.run(null);
fail("1.0");
} catch (RuntimeException e) {
- //expected
+ // expected
}
}
@@ -120,7 +70,7 @@
* Ensures that exceptions are propagated when the safe runner re-throws it
*/
public void testRethrow() {
- boolean caught = false;
+ IllegalArgumentException caughtException = null;
try {
SafeRunner.run(new ISafeRunnable() {
@Override
@@ -136,9 +86,64 @@
}
});
} catch (IllegalArgumentException e) {
- caught = true;
+ caughtException = e;
}
- assertTrue("1.0", caught);
+ assertNotNull("Cathed exception expected.", caughtException);
}
-}
+
+ public void testWithResult() {
+ assertEquals("TestRun", SafeRunner.run(() -> "TestRun"));
+ }
+
+ public void testWithResultReturnsNullOnException() {
+ ISafeRunnableWithResult<String> code = () -> {
+ throw new IllegalArgumentException();
+ };
+ assertNull(SafeRunner.run(code));
+ }
+
+ private void assertExceptionRethrown(Throwable current) {
+ Throwable caughtException = null;
+ try {
+ SafeRunner.run(new ISafeRunnable() {
+
+ @Override
+ public void run() throws Exception {
+ if (current instanceof Exception) {
+ throw (Exception) current;
+ } else if (current instanceof Error) {
+ throw (Error) current;
+ }
+ }
+ });
+ } catch (Throwable t) {
+ caughtException = t;
+ }
+ assertEquals("Unexpected exception.", current, caughtException);
+ }
+
+ private void assertExceptionHandled(Throwable throwable) {
+ final Throwable[] handled = new Throwable[1];
+ try {
+ SafeRunner.run(new ISafeRunnable() {
+ @Override
+ public void handleException(Throwable exception) {
+ handled[0] = exception;
+ }
+
+ @Override
+ public void run() throws Exception {
+ if (throwable instanceof Exception) {
+ throw (Exception) throwable;
+ } else if (throwable instanceof Error) {
+ throw (Error) throwable;
+ }
+ }
+ });
+ } catch (Throwable t) {
+ fail("Exception unexpectedly caught.", t);
+ }
+ assertEquals("Unexpected exception.", throwable, handled[0]);
+ }
+}
\ No newline at end of file
diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java
new file mode 100644
index 0000000..1122fa0
--- /dev/null
+++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java
@@ -0,0 +1,48 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Marcus Höpfner and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Marcus Höpfner - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.core.runtime;
+
+/**
+ * Safe runnables represent blocks of code and associated exception
+ * handlers. They are typically used when a plug-in needs to call some
+ * untrusted code (e.g., code contributed by another plug-in via an
+ * extension). In contradiction to {@link ISafeRunnable} this runnable is able to return a result.
+ * <p>
+ * This interface can be used without OSGi running.
+ * </p><p>
+ * Clients may implement this interface.
+ * </p>
+ * @param <T> the type of the result
+ * @see SafeRunner#run(ISafeRunnableWithResult)
+ *
+ * @since 3.11
+ */
+@FunctionalInterface
+public interface ISafeRunnableWithResult<T> extends ISafeRunnable {
+ @Override
+ default void run() throws Exception {
+ runWithResult();
+ }
+
+ /**
+ * Runs this runnable and returns the result. Any exceptions thrown from this method will
+ * be logged by the caller and passed to this runnable's
+ * {@link #handleException} method.
+ * @return the result
+ *
+ * @exception Exception if a problem occurred while running this method
+ * @see SafeRunner#run(ISafeRunnable)
+ */
+ public T runWithResult() throws Exception;
+}
\ No newline at end of file
diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java
index 3d6861f..0fc230b 100644
--- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java
+++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java
@@ -28,9 +28,9 @@
public final class SafeRunner {
/**
- * Runs the given runnable in a protected mode. Exceptions
+ * Runs the given runnable in a protected mode. Exceptions
* thrown in the runnable are logged and passed to the runnable's
- * exception handler. Such exceptions are not rethrown by this method.
+ * exception handler. Such exceptions are not rethrown by this method.
* <p>
* In addition to catching all {@link Exception} types, this method also catches certain {@link Error}
* types that typically result from programming errors in the code being executed.
@@ -48,29 +48,66 @@
}
}
- private static void handleException(ISafeRunnable code, Throwable e) {
- if (!(e instanceof OperationCanceledException)) {
- // try to obtain the correct plug-in id for the bundle providing the safe runnable
- Activator activator = Activator.getDefault();
- String pluginId = null;
- if (activator != null)
- pluginId = activator.getBundleId(code);
- if (pluginId == null)
- pluginId = IRuntimeConstants.PI_COMMON;
- String message = NLS.bind(CommonMessages.meta_pluginProblems, pluginId);
- IStatus status;
- if (e instanceof CoreException) {
- status = new MultiStatus(pluginId, IRuntimeConstants.PLUGIN_ERROR, message, e);
- ((MultiStatus) status).merge(((CoreException) e).getStatus());
- } else {
- status = new Status(IStatus.ERROR, pluginId, IRuntimeConstants.PLUGIN_ERROR, message, e);
- }
- // Make sure user sees the exception: if the log is empty, log the exceptions on stderr
- if (!RuntimeLog.isEmpty())
- RuntimeLog.log(status);
- else
- e.printStackTrace();
+ /**
+ * Runs the given runnable in a protected mode and returns the result given by the runnable. Exceptions
+ * thrown in the runnable are logged and passed to the runnable's
+ * exception handler. Such exceptions are not rethrown by this method, instead null is returned.
+ * <p>
+ * In addition to catching all {@link Exception} types, this method also catches certain {@link Error}
+ * types that typically result from programming errors in the code being executed.
+ * Severe errors that are not generally safe to catch are not caught by this method.
+ * </p>
+ * @param <T> the result type
+ *
+ * @param code the runnable to run
+ * @return the result
+ *
+ * @since 3.11
+ */
+ public static <T> T run(ISafeRunnableWithResult<T> code) {
+ Assert.isNotNull(code);
+ try {
+ return code.runWithResult();
+ } catch (Exception | LinkageError | AssertionError e) {
+ handleException(code, e);
+ return null;
}
- code.handleException(e);
}
-}
+
+ private static void handleException(ISafeRunnable code, Throwable exception) {
+ if (!(exception instanceof OperationCanceledException)) {
+ String pluginId = getBundleIdOfSafeRunnable(code);
+ IStatus status = convertToStatus(exception, pluginId);
+ makeSureUserSeesException(exception, status);
+ }
+ code.handleException(exception);
+ }
+
+ private static void makeSureUserSeesException(Throwable exception, IStatus status) {
+ if (RuntimeLog.isEmpty()) {
+ exception.printStackTrace();
+ } else {
+ RuntimeLog.log(status);
+ }
+ }
+
+ private static String getBundleIdOfSafeRunnable(ISafeRunnable code) {
+ Activator activator = Activator.getDefault();
+ String pluginId = null;
+ if (activator != null)
+ pluginId = activator.getBundleId(code);
+ if (pluginId == null)
+ return IRuntimeConstants.PI_COMMON;
+ return pluginId;
+ }
+
+ private static IStatus convertToStatus(Throwable exception, String pluginId) {
+ String message = NLS.bind(CommonMessages.meta_pluginProblems, pluginId);
+ if (exception instanceof CoreException) {
+ MultiStatus status = new MultiStatus(pluginId, IRuntimeConstants.PLUGIN_ERROR, message, exception);
+ status.merge(((CoreException) exception).getStatus());
+ return status;
+ }
+ return new Status(IStatus.ERROR, pluginId, IRuntimeConstants.PLUGIN_ERROR, message, exception);
+ }
+}
\ No newline at end of file
