Inspector proxy: preserve ordering of messages from device (facebook#…

…43639)

Summary:

Currently, the `react-native/dev-middleware` inspector proxy intercepts `Debugger.scriptParsed` notifications from the target and replaces `sourceMapURL` with a data uri, via an async fetch from Metro. During this async fetch, other notifications from the debugger may pass through the proxy, which results in the frontend receiving them before `Debugger.scriptParsed`.

This reordering causes problems in breakpoint resolution and pausing, because `Debugger.breakpointResolved` and `Debugger.paused` events may reference `scriptId`s unknown to the frontend while the corresponding `Debugger.scriptParsed` is delayed. 

In particular, breakpoint UI state and backend state can fall out of sync, and breakpoints hit may open to the incorrect source location.

This diff modifies the proxy to use a simple per-target promise queue to ensure messages are handled in the order they were received from the target.

Changelog:
[General][Fixed] Fix breakpoints opening to incorrect location or disappearing from debugger frontend UI.

Differential Revision: D55200617

packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js

@@ -95,6 +95,52 @@ describe.each(['HTTP', 'HTTPS'])(
       }
     });
 
+    test('async source map fetching does not reorder events', async () => {
+      serverRef.app.use(
+        '/source-map',
+        serveStaticJson({
+          version: 3,
+          // Mojibake insurance.
+          file: '\u2757.js',
+        }),
+      );
+      const {device, debugger_} = await createAndConnectTarget(
+        serverRef,
+        autoCleanup.signal,
+        {
+          app: 'bar-app',
+          id: 'page1',
+          title: 'bar-title',
+          vm: 'bar-vm',
+        },
+      );
+      try {
+        await Promise.all([
+          sendFromTargetToDebugger(device, debugger_, 'page1', {
+            method: 'Debugger.scriptParsed',
+            params: {
+              sourceMapURL: `${serverRef.serverBaseUrl}/source-map`,
+            },
+          }),
+          sendFromTargetToDebugger(device, debugger_, 'page1', {
+            method: 'Debugger.aSubsequentEvent',
+          }),
+        ]);
+        expect(debugger_.handle).toHaveBeenNthCalledWith(1, {
+          method: 'Debugger.scriptParsed',
+          params: {
+            sourceMapURL: expect.stringMatching(/^data:/),
+          },
+        });
+        expect(debugger_.handle).toHaveBeenNthCalledWith(2, {
+          method: 'Debugger.aSubsequentEvent',
+        });
+      } finally {
+        device.close();
+        debugger_.close();
+      }
+    });
+
     test('handling of failure to fetch source map', async () => {
       const {device, debugger_} = await createAndConnectTarget(
         serverRef,

packages/dev-middleware/src/inspector-proxy/Device.js

@@ -76,6 +76,11 @@ export default class Device {
   // Package name of the app.
   #app: string;
 
+  // Sequences async processing of messages from device to preserve order. Only
+  // necessary while we need to accommodate #processMessageFromDeviceLegacy's
+  // async fetch.
+  #messageFromDeviceQueue: Promise<void> = Promise.resolve();
+
   // Stores socket connection between Inspector Proxy and device.
   #deviceSocket: WS;
 
@@ -135,20 +140,26 @@ export default class Device {
 
     // $FlowFixMe[incompatible-call]
     this.#deviceSocket.on('message', (message: string) => {
-      const parsedMessage = JSON.parse(message);
-      if (parsedMessage.event === 'getPages') {
-        // There's a 'getPages' message every second, so only show them if they change
-        if (message !== this.#lastGetPagesMessage) {
-          debug(
-            '(Debugger)    (Proxy) <- (Device), getPages ping has changed: ' +
-              message,
-          );
-          this.#lastGetPagesMessage = message;
-        }
-      } else {
-        debug('(Debugger)    (Proxy) <- (Device): ' + message);
-      }
-      this.#handleMessageFromDevice(parsedMessage);
+      this.#messageFromDeviceQueue = this.#messageFromDeviceQueue.then(
+        async () => {
+          const parsedMessage = JSON.parse(message);
+          if (parsedMessage.event === 'getPages') {
+            // There's a 'getPages' message every second, so only show them if they change
+            if (message !== this.#lastGetPagesMessage) {
+              debug(
+                '(Debugger)    (Proxy) <- (Device), getPages ping has changed: ' +
+                  message,
+              );
+              this.#lastGetPagesMessage = message;
+            }
+          } else {
+            debug('(Debugger)    (Proxy) <- (Device): ' + message);
+          }
+          if (parsedMessage.event === 'wrappedEvent') {
+          }
+          await this.#handleMessageFromDevice(parsedMessage);
+        },
+      );
     });
     // Sends 'getPages' request to device every PAGES_POLLING_INTERVAL milliseconds.
     this.#pagesPollingIntervalId = setInterval(
@@ -395,7 +406,7 @@ export default class Device {
   // In the future more logic will be added to this method for modifying
   // some of the messages (like updating messages with source maps and file
   // locations).
-  #handleMessageFromDevice(message: MessageFromDevice) {
+  async #handleMessageFromDevice(message: MessageFromDevice) {
     if (message.event === 'getPages') {
       this.#pages = new Map(
         message.payload.map(({capabilities, ...page}) => [
@@ -498,16 +509,13 @@ export default class Device {
           return;
         }
 
-        // Wrapping just to make flow happy :)
-        // $FlowFixMe[unused-promise]
-        this.#processMessageFromDeviceLegacy(
+        await this.#processMessageFromDeviceLegacy(
           parsedPayload,
           debuggerConnection,
           pageId,
-        ).then(() => {
-          const messageToSend = JSON.stringify(parsedPayload);
-          debuggerSocket.send(messageToSend);
-        });
+        );
+        const messageToSend = JSON.stringify(parsedPayload);
+        debuggerSocket.send(messageToSend);
       } else {
         debuggerSocket.send(message.payload.wrappedEvent);
       }