From 736be5b35b7d40243754b894cc5bbfa0e6846049 Mon Sep 17 00:00:00 2001 From: nikneym Date: Thu, 17 Sep 2026 16:08:30 +0300 Subject: [PATCH] `cdp`: answer inspector commands on the session that sent them Playwright's `browserContext.newCDPSession` must get its response on that session, not the primary one. Playwright keys its pending callbacks by session, finds none for that id, and throws "Assertion error", which takes the whole process down. Addresses #1838 and #1839. --- src/server/cdp/CDP.zig | 41 ++++++++++++++++++++++-------- src/server/cdp/domains/runtime.zig | 28 +++++++++++++++++++- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/server/cdp/CDP.zig b/src/server/cdp/CDP.zig index cf1dd2e84..93052b35b 100644 --- a/src/server/cdp/CDP.zig +++ b/src/server/cdp/CDP.zig @@ -470,6 +470,7 @@ pub const BrowserContext = struct { // entries evicted by resetFrame can linger harmlessly until reset. set_child_nodes_sent: std.AutoHashMapUnmanaged(NodeRegistry.Id, void) = .empty, + inspector_call_sessions: std.AutoHashMapUnmanaged(i64, []const u8) = .empty, inspector_session: *js.Inspector.Session, isolated_worlds: std.ArrayList(*IsolatedWorld), @@ -622,6 +623,7 @@ pub const BrowserContext = struct { self.node_registry.deinit(); self.node_search_list.deinit(); self.set_child_nodes_sent.deinit(self.cdp.allocator); + self.inspector_call_sessions.deinit(self.cdp.allocator); // Session.deinit (called via closeSession above) already cleared this // notification off any ownerless CorsGate/RobotsGate transfers. @@ -1174,13 +1176,31 @@ pub const BrowserContext = struct { } } - pub fn callInspector(self: *const BrowserContext, msg: []const u8) void { - self.inspector_session.send(msg); + /// Forwards `cmd`'s raw JSON to the V8 inspector, which answers through onInspectorResponse. + pub fn callInspector(self: *BrowserContext, cmd: *const Command) !void { + try self.trackInspectorCall(cmd); + self.inspector_session.send(cmd.input.json); self.session.browser.env.runMicrotasks(); } - pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void { - sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg) catch |err| { + // Remembers which non-primary session `cmd` came from, so that its response + // can be stamped with that session. + fn trackInspectorCall(self: *BrowserContext, cmd: *const Command) !void { + const id = cmd.input.id orelse return; + const input_session_id = cmd.input.session_id orelse return; + if (self.session_id) |primary| { + if (std.mem.eql(u8, primary, input_session_id)) { + return; + } + } + const session_id = self.cdp.resolveSessionId(input_session_id) orelse return; + try self.inspector_call_sessions.put(self.cdp.allocator, id, session_id); + } + + pub fn onInspectorResponse(ctx: *anyopaque, call_id: u32, msg: []const u8) void { + const self: *BrowserContext = @ptrCast(@alignCast(ctx)); + const session_id = self.inspector_call_sessions.fetchRemove(@intCast(call_id)); + sendInspectorMessage(self, msg, if (session_id) |kv| kv.value else null) catch |err| { log.err(.cdp, "send inspector response", .{ .err = err }); }; } @@ -1197,16 +1217,17 @@ pub const BrowserContext = struct { log.debug(.cdp, "inspector event", .{ .method = method }); } - sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg) catch |err| { + sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg, null) catch |err| { log.err(.cdp, "send inspector event", .{ .err = err }); }; } - // This is hacky x 2. First, we create the JSON payload by gluing our - // session_id onto it. Second, we're much more client/websocket aware than - // we should be. - fn sendInspectorMessage(self: *BrowserContext, msg: []const u8) !void { - const session_id = self.session_id orelse { + // This is hacky x 2. First, we create the JSON payload by gluing a + // session_id onto it: `explicit_session_id` (the session a response + // belongs to) or else the primary session (all events). Second, we're + // much more client/websocket aware than we should be. + fn sendInspectorMessage(self: *BrowserContext, msg: []const u8, explicit_session_id: ?[]const u8) !void { + const session_id = explicit_session_id orelse self.session_id orelse { // We no longer have an active session. What should we do // in this case? return; diff --git a/src/server/cdp/domains/runtime.zig b/src/server/cdp/domains/runtime.zig index e36f1466e..8e4d4df5f 100644 --- a/src/server/cdp/domains/runtime.zig +++ b/src/server/cdp/domains/runtime.zig @@ -80,7 +80,7 @@ fn sendInspector(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; // the result to return is handled directly by the inspector. - bc.callInspector(cmd.input.json); + try bc.callInspector(cmd); } // Object arguments stay remote handles; serializing them would execute page JS. @@ -180,6 +180,32 @@ test "cdp.runtime: inspector-handled methods pass through" { try ctx.expectSentResult(null, .{ .id = 52 }); } +// Playwright's browserContext.newCDPSession attaches a second session to the +// page and sends Runtime commands through it. The inspector's answer must +// carry that session's id: the driver keys its pending callbacks by session +// and asserts on a response it can't match (lightpanda-io/browser#1838). +test "cdp.runtime: inspector responses go to the session that sent the command" { + var ctx = try testing.context(); + defer ctx.deinit(); + + const bc = try ctx.loadBrowserContext(.{ .id = "BID-RT2", .url = "hi.html", .target_id = "FID-0000000RT2".*, .session_id = "SID-PRIMARY" }); + try bc.attached_sessions.append(bc.arena, .{ .id = "SID-AUX", .parent_id = null }); + + try ctx.processMessage(.{ .id = 60, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "1 + 1", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 2, .description = "2" } }, .{ .id = 60, .session_id = "SID-AUX" }); + try testing.expectEqual(0, bc.inspector_call_sessions.count()); + + // The primary session keeps working as before. + try ctx.processMessage(.{ .id = 61, .method = "Runtime.evaluate", .sessionId = "SID-PRIMARY", .params = .{ .expression = "2 + 2", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 4, .description = "4" } }, .{ .id = 61, .session_id = "SID-PRIMARY" }); + + // A response the inspector produces after the dispatch (awaitPromise + // answers from a microtask) still finds its session. + try ctx.processMessage(.{ .id = 62, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "Promise.resolve('late')", .awaitPromise = true, .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "string", .value = "late" } }, .{ .id = 62, .session_id = "SID-AUX" }); + try testing.expectEqual(0, bc.inspector_call_sessions.count()); +} + test "cdp.runtime: consoleAPICalled type matches the console method" { testing.silenceLog(&.{.js});