From f5f7939a7dc01aa63b38cc8fdffa4167104db468 Mon Sep 17 00:00:00 2001 From: Amit Davidi Date: Tue, 25 Feb 2020 17:01:17 +0200 Subject: [PATCH] Make exceptions more descriptively reported to the user --- .../src/main/java/com/wix/detox/Detox.java | 6 +++++- .../java/com/wix/detox/DetoxActionHandlers.kt | 11 +++++----- .../java/com/wix/detox/DetoxCrashHandler.kt | 2 +- .../com/wix/detox/DetoxActionHandlersSpec.kt | 15 +++++++++----- .../main/java/com/example/NativeModule.java | 7 +++++++ detox/test/e2e/19.crash-handling.test.js | 20 ++++++++++++++++--- detox/test/src/app.js | 9 ++++++++- 7 files changed, 54 insertions(+), 16 deletions(-) diff --git a/detox/android/detox/src/main/java/com/wix/detox/Detox.java b/detox/android/detox/src/main/java/com/wix/detox/Detox.java index a9ec0339f6..6f0f9ec956 100644 --- a/detox/android/detox/src/main/java/com/wix/detox/Detox.java +++ b/detox/android/detox/src/main/java/com/wix/detox/Detox.java @@ -71,6 +71,7 @@ *

If not set, then Detox tests are no ops. So it's safe to mix it with other tests.

*/ public final class Detox { + private static final String LOG_TAG = "Detox"; private static final String LAUNCH_ARGS_KEY = "launchArgs"; private static final String DETOX_URL_OVERRIDE_ARG = "detoxURLOverride"; private static final long ACTIVITY_LAUNCH_TIMEOUT = 10000L; @@ -123,9 +124,12 @@ public static void runTests(ActivityTestRule activityTestRule, @NonNull final Co // Kicks off another thread and attaches a Looper to that. // The goal is to keep the test thread intact, // as Loopers can't run on a thread twice. - Thread t = new Thread(new Runnable() { + final Thread t = new Thread(new Runnable() { @Override public void run() { + Thread thread = Thread.currentThread(); + Log.i(LOG_TAG, "Detox thread starting (" + thread.getName() + ")"); + Looper.prepare(); new DetoxManager(context).start(); Looper.loop(); diff --git a/detox/android/detox/src/main/java/com/wix/detox/DetoxActionHandlers.kt b/detox/android/detox/src/main/java/com/wix/detox/DetoxActionHandlers.kt index 5848af4fed..2c173ff1f4 100644 --- a/detox/android/detox/src/main/java/com/wix/detox/DetoxActionHandlers.kt +++ b/detox/android/detox/src/main/java/com/wix/detox/DetoxActionHandlers.kt @@ -11,7 +11,7 @@ import org.json.JSONException import org.json.JSONObject import java.lang.reflect.InvocationTargetException -private const val LOG_TAG = "DetoxManager" +private const val LOG_TAG = "DetoxActionHandlers" interface DetoxActionHandler { fun handle(params: String, messageId: Long) @@ -41,9 +41,10 @@ class ReactNativeReloadActionHandler( } } -class InvokeActionHandler( +class InvokeActionHandler @JvmOverloads constructor( private val methodInvocation: MethodInvocation, - private val wsClient: WebSocketClient) + private val wsClient: WebSocketClient, + private val errorParser: (e: Throwable?) -> String = Log::getStackTraceString) : DetoxActionHandler { override fun handle(params: String, messageId: Long) { @@ -52,10 +53,10 @@ class InvokeActionHandler( wsClient.sendAction("invokeResult", mapOf("result" to invocationResult), messageId) } catch (e: InvocationTargetException) { Log.e(LOG_TAG, "Exception", e) - wsClient.sendAction("error", mapOf("error" to e.targetException?.message), messageId) + wsClient.sendAction("error", mapOf("error" to "${errorParser(e.targetException)}\nCheck device logs for full details!\n"), messageId) } catch (e: Exception) { Log.i(LOG_TAG, "Test exception", e) - wsClient.sendAction("testFailed", mapOf("details" to e.message), messageId) + wsClient.sendAction("testFailed", mapOf("details" to "${errorParser(e)}\nCheck device logs for full details!\n"), messageId) } } } diff --git a/detox/android/detox/src/main/java/com/wix/detox/DetoxCrashHandler.kt b/detox/android/detox/src/main/java/com/wix/detox/DetoxCrashHandler.kt index 7d354db72d..ab097517bf 100644 --- a/detox/android/detox/src/main/java/com/wix/detox/DetoxCrashHandler.kt +++ b/detox/android/detox/src/main/java/com/wix/detox/DetoxCrashHandler.kt @@ -7,7 +7,7 @@ class DetoxCrashHandler(private val wsClient: WebSocketClient) { Thread.setDefaultUncaughtExceptionHandler { thread, exception -> Log.e(LOG_TAG, "Crash detected!!! thread=${thread.name} (${thread.id})") - val crashInfo = mapOf("errorDetails" to "@Thread ${thread.name}(${thread.id}):\n${Log.getStackTraceString(exception)}") + val crashInfo = mapOf("errorDetails" to "@Thread ${thread.name}(${thread.id}):\n${Log.getStackTraceString(exception)}\nCheck device logs for full details!") wsClient.sendAction(ACTION_NAME, crashInfo, MESSAGE_ID) } } diff --git a/detox/android/detox/src/test/java/com/wix/detox/DetoxActionHandlersSpec.kt b/detox/android/detox/src/test/java/com/wix/detox/DetoxActionHandlersSpec.kt index e8291b3d5a..68101e2315 100644 --- a/detox/android/detox/src/test/java/com/wix/detox/DetoxActionHandlersSpec.kt +++ b/detox/android/detox/src/test/java/com/wix/detox/DetoxActionHandlersSpec.kt @@ -14,6 +14,8 @@ import java.lang.reflect.InvocationTargetException import java.util.* import java.util.concurrent.Executors +val mockErrorParser = { error: Throwable? -> "mockErrorParser($error)" } + object DetoxActionHandlersSpec : Spek({ describe("Action handlers") { val params = "{\"mock\": \"params\"}" @@ -94,7 +96,7 @@ object DetoxActionHandlersSpec : Spek({ describe("Invoke actions") { lateinit var methodInvocationMock: MethodInvocation - fun uut() = InvokeActionHandler(methodInvocationMock, wsClient) + fun uut() = InvokeActionHandler(methodInvocationMock, wsClient, mockErrorParser) beforeEachTest { methodInvocationMock = mock() @@ -125,24 +127,27 @@ object DetoxActionHandlersSpec : Spek({ } it("should handle runtime errors") { - whenever(methodInvocationMock.invoke(isA())).thenThrow(IllegalStateException("mock-error-reason")) + val exception = IllegalStateException("mock-error-reason") + whenever(methodInvocationMock.invoke(isA())).thenThrow(exception) + uut().handle(params, messageId) verify(wsClient).sendAction( eq("testFailed"), - argThat { size == 1 && this["details"] == "mock-error-reason" }, + argThat { size == 1 && this["details"] == "mockErrorParser($exception)\nCheck device logs for full details!\n" }, eq(messageId)) } it("should handle an InvocationTargetException") { - val exception = InvocationTargetException(Exception("mock-error-reason")) + val targetException = Exception("mock-error-reason") + val exception = InvocationTargetException(targetException) whenever(methodInvocationMock.invoke(isA())).thenThrow(exception) uut().handle(params, messageId) verify(wsClient).sendAction( eq("error"), - argThat { size == 1 && this["error"] == "mock-error-reason" }, + argThat { size == 1 && this["error"] == "mockErrorParser(${targetException})\nCheck device logs for full details!\n" }, eq(messageId)) } } diff --git a/detox/test/android/app/src/main/java/com/example/NativeModule.java b/detox/test/android/app/src/main/java/com/example/NativeModule.java index 932d626a70..a635a6a706 100644 --- a/detox/test/android/app/src/main/java/com/example/NativeModule.java +++ b/detox/test/android/app/src/main/java/com/example/NativeModule.java @@ -112,6 +112,13 @@ public void chokeMainThread() { }); } + @ReactMethod + public void crashMainThread() { + reactContext.getCurrentActivity().runOnUiThread(() -> { + throw new RuntimeException("Simulated crash (native)"); + }); + } + /** * Implementation note: For this purpose, a simpler RN API exists in the same class - * {@link ReactFindViewUtil#addViewListener(ReactFindViewUtil.OnViewFoundListener)}. diff --git a/detox/test/e2e/19.crash-handling.test.js b/detox/test/e2e/19.crash-handling.test.js index 927da6be24..750bf4129b 100644 --- a/detox/test/e2e/19.crash-handling.test.js +++ b/detox/test/e2e/19.crash-handling.test.js @@ -13,7 +13,7 @@ describe('Crash Handling', () => { try { await element(by.text('Crash')).tap(); await element(by.text('Crash')).tap(); - } catch (ex) { + } catch (ex) { // Note: exception will be logged as an APP_CRASH event failed = true; } @@ -25,12 +25,26 @@ describe('Crash Handling', () => { await expect(element(by.text('Sanity'))).toBeVisible(); }); + it(':android: should throw error upon invoke crash', async () => { + await device.reloadReactNative(); + let failed = false; + + try { + await element(by.text('UI Crash')).tap(); + } catch (ex) { + console.error(ex); // Log explicitly or it wouldn't show + failed = true; + } + + if (!failed) throw new Error('Test should have thrown an error, but did not'); + }); + it(':android: Should throw error upon app bootstrap crash', async () => { let failed = false; try { await device.launchApp({ newInstance: true, launchArgs: { detoxAndroidCrashingActivity: true }}); - } catch (e) { - console.error(e); + } catch (ex) { + console.error(ex); // Log explicitly or it wouldn't show failed = true; } diff --git a/detox/test/src/app.js b/detox/test/src/app.js index 406bd9ccfd..7673d568ff 100644 --- a/detox/test/src/app.js +++ b/detox/test/src/app.js @@ -113,7 +113,14 @@ class example extends Component { {this.renderButton('Crash', () => { - throw new Error('Simulated Crash') + // Note: this crashes the native-modules thread (and thus an *uncaught* exception, on Android). + throw new Error('Simulated Crash'); + })} + {isAndroid && | } + {isAndroid && this.renderButton('UI Crash', () => { + // Killing main-thread while handling a tap will evidently cause + // the tap-action itself to fail and thus for an error to be responded + NativeModule.crashMainThread(); })} {isAndroid && | } {isAndroid && this.renderButton('ANR', () => {