Repository: wix/Detox
Pull request #4376 (2024-02-20 15:12:20)

fix: communicate crash errors more clearly
## Description

- This pull request addresses recurring internal support tickets where our users seem to use the following pattern:

```js
try {
 await tapOnSomething();
} finally {
 await tapOnSomethingElse();
}
```

If `await tapOnSomething()` makes the application crash, the consequent action, `await tapOnSomethingElse()` executes when no app is running. This flow results into misleading errors like this:

```
Detox can't seem to connect to the test app(s)!

HINT: 

Have you forgotten to call 'device.launchApp()' in the beginning of your test?
Refer to our troubleshooting guide, for full details: https://wix.github.io/Detox/docs/troubleshooting/running-tests#tests-execution-hangs

---
The following package could not be delivered:

{
  type: 'invoke',
  params: [Object],
  messageId: 5
}
```

So, this PR improves crash error reporting – the Detox client memorizes the last crash error and doesn't clear it until it sees that the app is again connected to the server.

This change will ensure rethrowing the same crash error consistently, regardless of how many nested try-catch or try-finally statements exist. 


Patch:
diff --git a/detox/src/client/Client.js b/detox/src/client/Client.js
index 4fcc31139..c4a25a8e5 100644
--- a/detox/src/client/Client.js
+++ b/detox/src/client/Client.js
@@ -137,6 +137,10 @@ class Client {
   }
 
   async sendAction(action) {
+    if (this._pendingAppCrash) {
+      throw this._pendingAppCrash;
+    }
+
     const { shouldQueryStatus, ...options } = this._inferSendOptions(action);
 
     return await (shouldQueryStatus
@@ -302,6 +306,8 @@ class Client {
   }
 
   _onAppConnected() {
+    this._pendingAppCrash = null;
+
     if (this._whenAppIsConnected.isPending()) {
       this._whenAppIsConnected.resolve();
     } else {
@@ -352,7 +358,6 @@ class Client {
     if (this._pendingAppCrash) {
       this._whenAppDisconnected.reject(this._pendingAppCrash);
       this._asyncWebSocket.rejectAll(this._pendingAppCrash);
-      this._pendingAppCrash = null;
     } else if (this._asyncWebSocket.hasPendingActions()) {
       const error = new DetoxRuntimeError('The app has unexpectedly disconnected from Detox server.');
       this._asyncWebSocket.rejectAll(error);


Test patch:
diff --git a/detox/src/client/Client.test.js b/detox/src/client/Client.test.js
index 1499ae754..69b02b9ae 100644
--- a/detox/src/client/Client.test.js
+++ b/detox/src/client/Client.test.js
@@ -695,7 +695,38 @@ describe('Client', () => {
       mockAws.mockEventCallback('appDisconnected');
       expect(mockAws.rejectAll.mock.calls[0][0]).toMatchSnapshot();
       expect(log.error).not.toHaveBeenCalled();
-      await expect(client.waitUntilDisconnected()).rejects.toThrowError('SIGSEGV whatever');
+    });
+
+    describe('after app exits', () => {
+      beforeEach(async () => {
+        jest.spyOn(client, 'terminateApp');
+
+        await client.connect();
+
+        mockAws.mockEventCallback('AppWillTerminateWithError', {
+          params: { errorDetails: 'SIGSEGV whatever' },
+        });
+
+        jest.advanceTimersByTime(5000);
+        await fastForwardAllPromises();
+        mockAws.mockEventCallback('appDisconnected');
+        await fastForwardAllPromises();
+      });
+
+      it('should reject pending and future requests', async () => {
+        // any future requests should be rejected
+        mockAws.mockResponse('invokeResult', { result: 'some_result' });
+        await expect(client.sendAction(new actions.Invoke(anInvocation()))).rejects.toThrowError('SIGSEGV whatever');
+
+        // pending requests should be rejected
+        await expect(client.waitUntilDisconnected()).rejects.toThrowError('SIGSEGV whatever');
+      });
+
+      it('should allow new requests after the app reconnects', async () => {
+        mockAws.mockEventCallback('appConnected');
+        mockAws.mockResponse('invokeResult', { result: 'some_result' });
+        await expect(client.sendAction(new actions.Invoke(anInvocation()))).resolves.not.toThrow();
+      });
     });
 
     it('should log errors if the app termination does not go well', async () => {
diff --git a/detox/test/e2e/19.crash-handling.test.js b/detox/test/e2e/19.crash-handling.test.js
index 0c1f12021..7eb0bba2b 100644
--- a/detox/test/e2e/19.crash-handling.test.js
+++ b/detox/test/e2e/19.crash-handling.test.js
@@ -18,7 +18,10 @@ describe('Crash Handling', () => {
   it('Should throw error upon internal app crash', async () => {
     await device.reloadReactNative();
     await expectToThrow(() => element(by.text('Crash')).tap(), 'The app has crashed');
-    await expectToThrow(() => element(by.text('Crash')).tap(), 'Detox can\'t seem to connect to the test app(s)!');
+  });
+
+  it('Should throw the same crash error even in the next test if the app was not relaunched', async () => {
+    await expectToThrow(() => element(by.text('Crash')).tap(), 'The app has crashed');
   });
 
   it('Should recover from app crash', async () => {
@@ -26,6 +29,16 @@ describe('Crash Handling', () => {
     await expect(element(by.text('Sanity'))).toBeVisible();
   });
 
+  it('Should print generic connectivity error when the app was terminated intentionally', async () => {
+    /**
+     * @issue https://github.com/wix/Detox/issues/4377
+     * @tag flaky
+     */
+    await device.terminateApp();
+    await new Promise((resolve) => setTimeout(resolve, 2000)); // see the issue for details
+    await expectToThrow(() => element(by.text('Crash')).tap(), 'Detox can\'t seem to connect to the test app(s)!');
+  });
+
   it('Should throw a detailed error upon early app crash', async () => {
     const error = await expectToThrow(
       () => relaunchAppWithArgs({ simulateEarlyCrash: true }),