Refactor server implementation for WebSockets
Summary: Standardize WS implementation for JS environments. Why do we need a separate server implementation for browsers? Browser targets cannot authenticate via the default certificate exchange flow. For browser targets we verify the origin instead. Moreover, for already forgotten reasons the initial implementation of the WS server for browsers used a different kind of message structure and added extra `connect`/`disconnect` messages. After examination, it seems the `connect`/`disconnect` flow is redundant. Major changes: 1. Updated class hierarchy for WS server implementations. 2. Updated browser WS server to support the modern and the legacy protocols. 3. Now a websocket connection with the device is closed on error. The idea is it is highly unlikely to handle any subsequent messages properly once we observe an error. It is better to bail and reconnect. What do you think? Reviewed By: mweststrate Differential Revision: D31532172 fbshipit-source-id: f86aa63a40efe4d5263353cc124fac8c63b80e45
This commit is contained in:
committed by
Facebook GitHub Bot
parent
6bd4a07286
commit
37498ad5a9
@@ -8,18 +8,18 @@
|
||||
*/
|
||||
|
||||
import {CertificateExchangeMedium} from '../utils/CertificateProvider';
|
||||
import {Logger} from 'flipper-common';
|
||||
import {
|
||||
ClientDescription,
|
||||
ClientQuery,
|
||||
isTest,
|
||||
GK,
|
||||
buildClientId,
|
||||
Logger,
|
||||
UninitializedClient,
|
||||
reportPlatformFailures,
|
||||
} from 'flipper-common';
|
||||
import CertificateProvider from '../utils/CertificateProvider';
|
||||
import {ClientConnection, ConnectionStatus} from './ClientConnection';
|
||||
import {UninitializedClient} from 'flipper-common';
|
||||
import {reportPlatformFailures} from 'flipper-common';
|
||||
import {EventEmitter} from 'events';
|
||||
import invariant from 'invariant';
|
||||
import DummyDevice from '../devices/DummyDevice';
|
||||
@@ -161,6 +161,7 @@ class ServerController extends EventEmitter implements ServerEventsListener {
|
||||
});
|
||||
|
||||
if (GK.get('comet_enable_flipper_connection')) {
|
||||
console.info('[conn] Browser server (ws) listening at port: ', 8333);
|
||||
this.browserServer = createBrowserServer(8333, this);
|
||||
}
|
||||
|
||||
@@ -187,6 +188,7 @@ class ServerController extends EventEmitter implements ServerEventsListener {
|
||||
onConnectionCreated(
|
||||
clientQuery: SecureClientQuery,
|
||||
clientConnection: ClientConnection,
|
||||
downgrade: boolean,
|
||||
): Promise<ClientDescription> {
|
||||
const {app, os, device, device_id, sdk_version, csr, csr_path, medium} =
|
||||
clientQuery;
|
||||
@@ -206,6 +208,7 @@ class ServerController extends EventEmitter implements ServerEventsListener {
|
||||
medium: transformedMedium,
|
||||
},
|
||||
{csr, csr_path},
|
||||
downgrade,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -334,6 +337,7 @@ class ServerController extends EventEmitter implements ServerEventsListener {
|
||||
connection: ClientConnection,
|
||||
query: ClientQuery & {medium: CertificateExchangeMedium},
|
||||
csrQuery: ClientCsrQuery,
|
||||
silentReplace?: boolean,
|
||||
): Promise<ClientDescription> {
|
||||
invariant(query, 'expected query');
|
||||
|
||||
@@ -411,7 +415,9 @@ class ServerController extends EventEmitter implements ServerEventsListener {
|
||||
connectionInfo.connection &&
|
||||
connectionInfo.connection !== connection
|
||||
) {
|
||||
connectionInfo.connection.close();
|
||||
if (!silentReplace) {
|
||||
connectionInfo.connection.close();
|
||||
}
|
||||
this.removeConnection(id);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user