Allow client-side device identifiers in inspector proxy (#991)

Summary:
Fixes #985

This allows devices connecting to the inspector proxy to provide a client-side unique string identifier. With this, we can pass along a unique identifier that doesn't change when reconnecting, e.g. after a hard crash.

It makes the overall debugging experience way more stable since you don't have to restart the debugger in between crashes or full restarts. Allowing users to keep the debugger open also lets the debugger "remember" certain things, such as set breakpoints.

> ⚠️ When the client doesn't specify this unique identifier, it still falls back to the incremental identifier (the `fallbackDeviceId` in this PR)

When a collision occurs, the old device's connection is closed. But, if both the device and app names are equal to the new device connection, the debugger connection is kept open.

## Next steps

If this is landed, we still need changes in React Native (the `&device=...` query param). I have no strong opinions on what identifier is used, but it should follow these rules:
- Should be unique per device or emulator/simulator (and thus, also per platform)
- Must not change when restarting the app
- Must be a string that's URL safe

On Android, I had good success using the [`Secure.ANDROID_ID`](https://developer.android.com/reference/android/provider/Settings.Secure#ANDROID_ID). It may change after a factory reset, which doesn't matter for our use case.

Pull Request resolved: #991

Test Plan:
- Create a new project, and enable building from source on both Android & iOS
- **android**: edit [this file](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java#L282-L289) in `node_modules/react-native` and add a hardcoded `&device=testingandroid` query param.
- **ios**: edit [this file](https://github.com/facebook/react-native/blob/main/packages/react-native/React/DevSupport/RCTInspectorDevServerHelper.mm#L43-L53) in `node_modules/react-naive` and add a hardcoded `&device=testingios` query param.
- Connect the debugger to the running app
- Force close the app, which should cause a "reconnect" warning in the debugger
- Open the app again, and press "reconnect" in the debugger
- _Due to the stable identifiers, the URL won't change and the above scenario should work fine_

Also test without these `&device=...` query param since that should work as it does now (with incremental identifiers).

Reviewed By: motiz88

Differential Revision: D46482492

Pulled By: huntie

fbshipit-source-id: 5d6e1e9c692d78ebfde8f4938041a8f381a1d48c

packages/metro-inspector-proxy/src/Device.js

@@ -55,7 +55,7 @@ const REACT_NATIVE_RELOADABLE_PAGE_ID = '-1';
  */
 class Device {
   // ID of the device.
-  _id: number;
+  _id: string;
 
   // Name of the device.
   _name: string;
@@ -90,7 +90,7 @@ class Device {
   _projectRoot: string;
 
   constructor(
-    id: number,
+    id: string,
     name: string,
     app: string,
     socket: typeof WS,
@@ -134,6 +134,10 @@ class Device {
     return this._name;
   }
 
+  getApp(): string {
+    return this._app;
+  }
+
   getPagesList(): Array<Page> {
     if (this._lastConnectedReactNativePage) {
       const reactNativeReloadablePage = {
@@ -212,6 +216,37 @@ class Device {
     };
   }
 
+  /**
+   * Handles cleaning up a duplicate device connection, by client-side device ID.
+   * 1. Checks if the same device is attempting to reconnect for the same app.
+   * 2. If not, close both the device and debugger socket.
+   * 3. If the debugger connection can be reused, close the device socket only.
+   *
+   * This allows users to reload the app, either as result of a crash, or manually
+   * reloading, without having to restart the debugger.
+   */
+  handleDuplicateDeviceConnection(newDevice: Device) {
+    if (
+      this._app !== newDevice.getApp() ||
+      this._name !== newDevice.getName()
+    ) {
+      this._deviceSocket.close();
+      this._debuggerConnection?.socket.close();
+    }
+
+    const oldDebugger = this._debuggerConnection;
+    this._debuggerConnection = null;
+
+    if (oldDebugger) {
+      oldDebugger.socket.removeAllListeners();
+      this._deviceSocket.close();
+      newDevice.handleDebuggerConnection(
+        oldDebugger.socket,
+        oldDebugger.pageId,
+      );
+    }
+  }
+
   // Handles messages received from device:
   // 1. For getPages responses updates local _pages list.
   // 2. All other messages are forwarded to debugger as wrappedEvent.

packages/metro-inspector-proxy/src/InspectorProxy.js

@@ -41,7 +41,7 @@ class InspectorProxy {
   _projectRoot: string;
 
   // Maps device ID to Device instance.
-  _devices: Map<number, Device>;
+  _devices: Map<string, Device>;
 
   // Internal counter for device IDs -- just gets incremented for each new device.
   _deviceCounter: number = 0;
@@ -111,7 +111,7 @@ class InspectorProxy {
   // Converts page information received from device into PageDescription object
   // that is sent to debugger.
   _buildPageDescription(
-    deviceId: number,
+    deviceId: string,
     device: Device,
     page: Page,
   ): PageDescription {
@@ -162,16 +162,31 @@ class InspectorProxy {
     // $FlowFixMe[value-as-type]
     wss.on('connection', async (socket: WS, req) => {
       try {
+        const fallbackDeviceId = String(this._deviceCounter++);
+
         const query = url.parse(req.url || '', true).query || {};
+        const deviceId = query.device || fallbackDeviceId;
         const deviceName = query.name || 'Unknown';
         const appName = query.app || 'Unknown';
-        const deviceId = this._deviceCounter++;
-        this._devices.set(
+
+        const oldDevice = this._devices.get(deviceId);
+        const newDevice = new Device(
           deviceId,
-          new Device(deviceId, deviceName, appName, socket, this._projectRoot),
+          deviceName,
+          appName,
+          socket,
+          this._projectRoot,
         );
 
-        debug(`Got new connection: device=${deviceName}, app=${appName}`);
+        if (oldDevice) {
+          oldDevice.handleDuplicateDeviceConnection(newDevice);
+        }
+
+        this._devices.set(deviceId, newDevice);
+
+        debug(
+          `Got new connection: name=${deviceName}, app=${appName}, device=${deviceId}`,
+        );
 
         socket.on('close', () => {
           this._devices.delete(deviceId);
@@ -206,7 +221,7 @@ class InspectorProxy {
           throw new Error('Incorrect URL - must provide device and page IDs');
         }
 
-        const device = this._devices.get(parseInt(deviceId, 10));
+        const device = this._devices.get(deviceId);
         if (device == null) {
           throw new Error('Unknown device with ID ' + deviceId);
         }