From c007d74af9335ba77f772fe82c77520bf31cde54 Mon Sep 17 00:00:00 2001
From: Pascal Hartig <realpassy@fb.com>
Date: Thu, 22 Apr 2021 07:44:17 -0700
Subject: [PATCH] Handle RSocket disconnects gracefully

Summary:
Tiny change, hours of debugging. Big thanks to jknoxville for
answering all my stupid questions.

Our number one "error" right now by users affected is

```
Unhandled Promise Rejection:
Error: RSocketTcpClient:
Socket closed unexpectedly.
```

Of course no stacktrace, because JavaScript and Promises.

The underlying problem is that RSocket keeps track of recently used
"receivers" which is every method that has sent a request. When someone disconnects their device or closes an emulator/simulator, the socket's `end` event is treated the same way any other connection error would be: https://github.com/rsocket/rsocket-js/blob/a85a4db0417717a8734b594e24de9c374eca2851/packages/rsocket-tcp-client/src/RSocketTcpClient.js#L74

This then causes "errors" like these to appear:
{F609810187}

This could previously be handled in plugin-code by wrapping all `client.call()` invocations in try/catch, which nobody did, meaning we'd get rejected promises everywhere. We can instead handle this centrally and properly disconnect the connection.

Changelog: Severed RSocket connections are no longer treated as an error in plugin code

Reviewed By: jknoxville

Differential Revision: D27910514

fbshipit-source-id: ea9c0726ab0e959b0eb4a5fca67ddaa04a6f1d14
---
 desktop/app/src/Client.tsx | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx
index 5e806f89d..c119b6f61 100644
--- a/desktop/app/src/Client.tsx
+++ b/desktop/app/src/Client.tsx
@@ -334,10 +334,9 @@ export default class Client extends EventEmitter {
     if (this.sdkVersion < 4) {
       return [];
     }
-    return await this.rawCall<{plugins: Plugins}>(
-      'getBackgroundPlugins',
-      false,
-    ).then((data) => data.plugins);
+    return this.rawCall<{plugins: Plugins}>('getBackgroundPlugins', false).then(
+      (data) => data.plugins,
+    );
   }
 
   // get the plugins, and update the UI
@@ -621,7 +620,10 @@ export default class Client extends EventEmitter {
             }
           },
           onError: (e) => {
-            reject(e);
+            // This is only called if the connection is dead. Not in expected
+            // and recoverable cases like a missing receiver/method.
+            this.disconnect();
+            reject(new Error('Connection disconnected: ' + e));
           },
         });
       } else {
@@ -734,7 +736,21 @@ export default class Client extends EventEmitter {
     params?: Object,
   ): Promise<Object> {
     return reportPluginFailures(
-      this.rawCall('execute', fromPlugin, {api, method, params}),
+      this.rawCall<Object>('execute', fromPlugin, {
+        api,
+        method,
+        params,
+      }).catch((err) => {
+        // We only throw errors if the connection is still alive
+        // as connection-related ones aren't recoverable from
+        // user code.
+        if (this.connected.get()) {
+          throw err;
+        }
+        // This effectively preserves the previous behavior
+        // of ignoring disconnection-related call failures.
+        return {};
+      }),
       `Call-${method}`,
       api,
     );