From ac10d5b2a31a2d5dfb279ce682da29d94710c245 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 5 Sep 2025 15:07:30 +0800 Subject: [PATCH] Don't assume that page events means the BrowserContext has a page CDP currently assumes that if we get a page-related notification (like a request interception, or page lifecycle event), then we must have a session and page. But, Target.detachFromTarget can remove the session from the BrowserContext while still having the page run (I wonder if we should stop the page at this point??). So, remove these assumptions and make sure we have a page/session in the handling of page events. --- src/cdp/domains/fetch.zig | 12 ++++++++---- src/cdp/domains/network.zig | 38 ++++++++++++------------------------- src/cdp/domains/page.zig | 28 +++++++++++++-------------- 3 files changed, 33 insertions(+), 45 deletions(-) diff --git a/src/cdp/domains/fetch.zig b/src/cdp/domains/fetch.zig index da2a7ddd..ea7d225a 100644 --- a/src/cdp/domains/fetch.zig +++ b/src/cdp/domains/fetch.zig @@ -179,8 +179,10 @@ fn arePatternsSupported(patterns: []RequestPattern) bool { } pub fn requestIntercept(arena: Allocator, bc: anytype, intercept: *const Notification.RequestIntercept) !void { - // unreachable because we _have_ to have a page. - const session_id = bc.session_id orelse unreachable; + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; + const target_id = bc.target_id orelse unreachable; // We keep it around to wait for modifications to the request. @@ -380,8 +382,10 @@ fn failRequest(cmd: anytype) !void { } pub fn requestAuthRequired(arena: Allocator, bc: anytype, intercept: *const Notification.RequestAuthRequired) !void { - // unreachable because we _have_ to have a page. - const session_id = bc.session_id orelse unreachable; + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; + const target_id = bc.target_id orelse unreachable; // We keep it around to wait for modifications to the request. diff --git a/src/cdp/domains/network.zig b/src/cdp/domains/network.zig index e713b103..c8a1788d 100644 --- a/src/cdp/domains/network.zig +++ b/src/cdp/domains/network.zig @@ -230,14 +230,10 @@ pub fn httpRequestFail(arena: Allocator, bc: anytype, msg: *const Notification.R } pub fn httpRequestStart(arena: Allocator, bc: anytype, msg: *const Notification.RequestStart) !void { - // Isn't possible to do a network request within a Browser (which our - // notification is tied to), without a page. - std.debug.assert(bc.session.page != null); + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; - var cdp = bc.cdp; - - // all unreachable because we _have_ to have a page. - const session_id = bc.session_id orelse unreachable; const target_id = bc.target_id orelse unreachable; const page = bc.session.currentPage() orelse unreachable; @@ -248,22 +244,17 @@ pub fn httpRequestStart(arena: Allocator, bc: anytype, msg: *const Notification. const transfer = msg.transfer; // We're missing a bunch of fields, but, for now, this seems like enough - try cdp.sendEvent("Network.requestWillBeSent", .{ .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{transfer.id}), .frameId = target_id, .loaderId = bc.loader_id, .documentUrl = DocumentUrlWriter.init(&page.url.uri), .request = TransferAsRequestWriter.init(transfer) }, .{ .session_id = session_id }); + try bc.cdp.sendEvent("Network.requestWillBeSent", .{ .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{transfer.id}), .frameId = target_id, .loaderId = bc.loader_id, .documentUrl = DocumentUrlWriter.init(&page.url.uri), .request = TransferAsRequestWriter.init(transfer) }, .{ .session_id = session_id }); } pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notification.ResponseHeaderDone) !void { - // Isn't possible to do a network request within a Browser (which our - // notification is tied to), without a page. - std.debug.assert(bc.session.page != null); - - var cdp = bc.cdp; - - // all unreachable because we _have_ to have a page. - const session_id = bc.session_id orelse unreachable; + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; const target_id = bc.target_id orelse unreachable; // We're missing a bunch of fields, but, for now, this seems like enough - try cdp.sendEvent("Network.responseReceived", .{ + try bc.cdp.sendEvent("Network.responseReceived", .{ .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{msg.transfer.id}), .loaderId = bc.loader_id, .frameId = target_id, @@ -272,16 +263,11 @@ pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notific } pub fn httpRequestDone(arena: Allocator, bc: anytype, msg: *const Notification.RequestDone) !void { - // Isn't possible to do a network request within a Browser (which our - // notification is tied to), without a page. - std.debug.assert(bc.session.page != null); + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; - var cdp = bc.cdp; - - // all unreachable because we _have_ to have a page. - const session_id = bc.session_id orelse unreachable; - - try cdp.sendEvent("Network.loadingFinished", .{ + try bc.cdp.sendEvent("Network.loadingFinished", .{ .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{msg.transfer.id}), .encodedDataLength = msg.transfer.bytes_received, }, .{ .session_id = session_id }); diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 1394953a..4495a474 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -162,19 +162,17 @@ fn navigate(cmd: anytype) !void { } pub fn pageNavigate(arena: Allocator, bc: anytype, event: *const Notification.PageNavigate) !void { - // I don't think it's possible that we get these notifications and don't - // have these things setup. - std.debug.assert(bc.session.page != null); - - var cdp = bc.cdp; + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; bc.loader_id = bc.cdp.loader_id_gen.next(); const loader_id = bc.loader_id; const target_id = bc.target_id orelse unreachable; - const session_id = bc.session_id orelse unreachable; bc.reset(); + var cdp = bc.cdp; const reason_: ?[]const u8 = switch (event.opts.reason) { .anchor => "anchorClick", .script => "scriptInitiated", @@ -292,16 +290,14 @@ pub fn pageCreated(bc: anytype, page: *Page) !void { } pub fn pageNavigated(bc: anytype, event: *const Notification.PageNavigated) !void { - // I don't think it's possible that we get these notifications and don't - // have these things setup. - std.debug.assert(bc.session.page != null); - - var cdp = bc.cdp; - const timestamp = event.timestamp; + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; const loader_id = bc.loader_id; const target_id = bc.target_id orelse unreachable; - const session_id = bc.session_id orelse unreachable; + const timestamp = event.timestamp; + var cdp = bc.cdp; // frameNavigated event try cdp.sendEvent("Page.frameNavigated", .{ .type = "Navigation", @@ -370,10 +366,12 @@ pub fn pageNetworkAlmostIdle(bc: anytype, event: *const Notification.PageNetwork } fn sendPageLifecycle(bc: anytype, name: []const u8, timestamp: u32) !void { + // detachTarget could be called, in which case, we still have a page doing + // things, but no session. + const session_id = bc.session_id orelse return; + const loader_id = bc.loader_id; const target_id = bc.target_id orelse unreachable; - const session_id = bc.session_id orelse unreachable; - return bc.cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{ .name = name, .frameId = target_id,