fix: only auto-attach to a target once

.github/workflows/puppeteer.yml

@@ -30,8 +30,6 @@ jobs:
         uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
         with:
           path: 'bidi'
-      - name: Set up FFmpeg
-        uses: FedericoCarboni/setup-ffmpeg@36c6454b5a2348e7794ba2d82a21506605921e3d # v3.0.0
       - name: Set up Node.js
         uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
         with:

src/bidiMapper/modules/cdp/CdpTargetManager.ts

@@ -43,6 +43,7 @@ const cdpToBidiTargetTypes = {
 export class CdpTargetManager {
   readonly #browserCdpClient: CdpClient;
   readonly #cdpConnection: CdpConnection;
+  readonly #targetIdsToBeIgnoredByAutoAttach = new Set<string>();
   readonly #selfTargetId: string;
   readonly #eventManager: EventManager;
 
@@ -70,6 +71,7 @@ export class CdpTargetManager {
   ) {
     this.#cdpConnection = cdpConnection;
     this.#browserCdpClient = browserCdpClient;
+    this.#targetIdsToBeIgnoredByAutoAttach.add(selfTargetId);
     this.#selfTargetId = selfTargetId;
     this.#eventManager = eventManager;
     this.#browsingContextStorage = browsingContextStorage;
@@ -149,20 +151,49 @@ export class CdpTargetManager {
     parentSessionCdpClient: CdpClient
   ) {
     const {sessionId, targetInfo} = params;
+
+    if (this.#selfTargetId !== targetInfo.targetId) {
+      // Mapper only needs one session per target. If we receive additional
+      // auto-attached sessions, that is very likely coming from custom CDP
+      // sessions.
+      if (this.#targetIdsToBeIgnoredByAutoAttach.has(targetInfo.targetId)) {
+        // Return to leave the session be unless it is a service worker.
+        if (targetInfo.type === 'service_worker' && params.waitingForDebugger) {
+          // Service workers are special case because they attach to the
+          // browser target and the page target during the regular
+          // auto-attach and might hang if the CDP session is not
+          // detached.
+          const targetCdpClient = this.#cdpConnection.getCdpClient(sessionId);
+          targetCdpClient
+            .sendCommand('Runtime.runIfWaitingForDebugger')
+            .then(() =>
+              parentSessionCdpClient.sendCommand(
+                'Target.detachFromTarget',
+                params
+              )
+            )
+            .catch((error) => this.#logger?.(LogType.debugError, error));
+        }
+        return;
+      }
+      this.#targetIdsToBeIgnoredByAutoAttach.add(targetInfo.targetId);
+    }
+
     const targetCdpClient = this.#cdpConnection.getCdpClient(sessionId);
 
     switch (targetInfo.type) {
       case 'page':
       case 'iframe': {
-        if (targetInfo.targetId === this.#selfTargetId) {
+        if (this.#selfTargetId === targetInfo.targetId) {
+          // break to detach from the self target.
           break;
         }
 
         const cdpTarget = this.#createCdpTarget(targetCdpClient, targetInfo);
         const maybeContext = this.#browsingContextStorage.findContext(
           targetInfo.targetId
         );
-        if (maybeContext) {
+        if (maybeContext && targetInfo.type === 'iframe') {
           // OOPiF.
           maybeContext.updateCdpTarget(cdpTarget);
         } else {

tests/service_worker/test_navigate.py

@@ -24,7 +24,7 @@
 }],
                          indirect=True)
 async def test_serviceWorker_acceptInsecureCertsCapability_respected(
-        websocket, context_id, url_bad_ssl, local_server_bad_ssl):
+        websocket, context_id, local_server_bad_ssl):
     service_worker_script = local_server_bad_ssl.url_200(
         content='', content_type='text/javascript')
     service_worker_page = local_server_bad_ssl.url_200(content=f"""<script>

tests/session/test_cdp.py

@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import asyncio
 from unittest.mock import ANY
 
 import pytest
@@ -158,3 +159,53 @@ async def test_cdp_wait_for_event(websocket, get_cdp_session_id, context_id):
             "session": session_id
         }
     })
+
+
+@pytest.mark.asyncio
+async def test_cdp_no_extraneous_events(websocket, get_cdp_session_id,
+                                        create_context, url_base):
+    new_context_id = await create_context()
+    await execute_command(
+        websocket, {
+            "method": "browsingContext.navigate",
+            "params": {
+                "url": url_base,
+                "wait": "complete",
+                "context": new_context_id
+            }
+        })
+
+    await subscribe(websocket, ["cdp"], [new_context_id])
+
+    session_id = await get_cdp_session_id(new_context_id)
+
+    id = await send_JSON_command(
+        websocket, {
+            "method": "cdp.sendCommand",
+            "params": {
+                "method": "Target.attachToTarget",
+                "params": {
+                    "targetId": new_context_id,
+                    "flatten": True
+                },
+                "session": session_id
+            }
+        })
+
+    events = []
+    event = await read_JSON_message(websocket)
+
+    session_id = None
+    with pytest.raises(asyncio.TimeoutError):
+        while True:
+            if 'id' in event and event['id'] == id:
+                session_id = event['result']['result']['sessionId']
+            if 'id' not in event:
+                events.append(event)
+            event = await asyncio.wait_for(read_JSON_message(websocket),
+                                           timeout=1.0)
+
+    for event in events:
+        if event['method'].startswith(
+                'cdp') and event['params']['session'] == session_id:
+            raise Exception("Unrelated CDP events detected")

tests/test_helpers.py

@@ -69,7 +69,10 @@ async def send_JSON_command(websocket, command: dict) -> int:
 
 
 async def read_JSON_message(websocket) -> dict:
-    return json.loads(await websocket.recv())
+    logging.info("calling websocket recv", exc_info=True)
+    result = json.loads(await websocket.recv())
+    logging.info("calling websocket recv: done", exc_info=True)
+    return result
 
 
 async def execute_command(websocket, command: dict, timeout: int = 5) -> dict:

tools/bidi-server.mjs

@@ -55,7 +55,7 @@ export function createLogFile(suffix) {
 export function parseCommandLineArgs() {
   return yargs(hideBin(process.argv))
     .usage(
-      `$0 [fileOrFolder]`,
+      `$0 [fileOrFolder...]`,
       `[CHANNEL=<local | stable | beta | canary | dev>] [DEBUG=*] [DEBUG_COLORS=<yes | no>] [LOG_DIR=logs] [NODE_OPTIONS=--unhandled-rejections=strict] [PORT=8080]`,
       (yargs) => {
         yargs.positional('fileOrFolder', {

tools/run-e2e.mjs

@@ -133,12 +133,12 @@ syncFileStreams.pipe(fileWriteStream);
 const serverProcess = createBiDiServerProcess();
 
 if (serverProcess.stderr) {
-  serverProcess.stdout.pipe(process.stdout);
   serverProcess.stderr.pipe(syncFileStreams);
 }
 
 if (serverProcess.stdout) {
   serverProcess.stdout.pipe(syncFileStreams);
+  serverProcess.stdout.pipe(process.stdout);
 }
 
 await matchLine(serverProcess).catch((error) => {
@@ -166,7 +166,11 @@ if (PYTEST_TOTAL_CHUNKS !== 1) {
 }
 
 if (argv.fileOrFolder) {
-  e2eArgs.push(argv.fileOrFolder);
+  e2eArgs.push(
+    ...(Array.isArray(argv.fileOrFolder)
+      ? argv.fileOrFolder
+      : [argv.fileOrFolder])
+  );
 }
 if (process.env.HEADLESS === 'false' && !argv.k) {
   e2eArgs.push('--ignore=tests/input');