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: a85a4db041/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
This commit is contained in:
Pascal Hartig
2021-04-22 07:44:17 -07:00
committed by Facebook GitHub Bot
parent 9e7a455910
commit c007d74af9

View File

@@ -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,
);