mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-10-09 04:42:30 -04:00
Don't resolve a navigation queued on a free connection as loaded
The easy-handle pool is shared by every client in the process. When other sessions hold all of it, a new navigation's request sits in pending_queue, but Runner resolved a frame still in `.pre` as soon as in-flight activity was zero, which ignores pending transfers. goto then reported success on an empty document. Wait on `activity.idle()` instead, and make a goto that times out before any response arrives a NavigationTimeout error rather than a soft timeout. Fixes #3636
This commit is contained in:
1 parent
8702e06167
commit
d77c6fb1ca
4 files changed
+32
-4
No files matched your search
@@ -231,7 +231,6 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa
|
|||||||
|
|
||||||
const activity = http_client.activity();
|
const activity = http_client.activity();
|
||||||
const total_http_activity = activity.http;
|
const total_http_activity = activity.http;
|
||||||
const total_network_activity = activity.total();
|
|
||||||
|
|
||||||
const network_idle = activity.idle();
|
const network_idle = activity.idle();
|
||||||
const is_done = browser.hasMacrotasks() == false and network_idle;
|
const is_done = browser.hasMacrotasks() == false and network_idle;
|
||||||
@@ -283,7 +282,8 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa
|
|||||||
condition.status = .complete;
|
condition.status = .complete;
|
||||||
},
|
},
|
||||||
.pre, .raw, .text, .image, .download => {
|
.pre, .raw, .text, .image, .download => {
|
||||||
if (total_network_activity == 0) {
|
// Includes pending: another client may hold every connection.
|
||||||
|
if (network_idle) {
|
||||||
condition.status = .complete;
|
condition.status = .complete;
|
||||||
} else {
|
} else {
|
||||||
want_http_tick = true;
|
want_http_tick = true;
|
||||||
|
|||||||
@@ -795,6 +795,7 @@ pub const ToolError = error{
|
|||||||
InvalidParams,
|
InvalidParams,
|
||||||
NodeNotFound,
|
NodeNotFound,
|
||||||
NavigationFailed,
|
NavigationFailed,
|
||||||
|
NavigationTimeout,
|
||||||
Cancelled,
|
Cancelled,
|
||||||
Timeout,
|
Timeout,
|
||||||
InternalError,
|
InternalError,
|
||||||
@@ -807,6 +808,7 @@ pub fn errorMessage(err: ToolError) []const u8 {
|
|||||||
return switch (err) {
|
return switch (err) {
|
||||||
error.NodeNotFound => "NodeNotFound: the selector or backendNodeId matched nothing on the current page. Re-inspect the page (tree/interactiveElements) for fresh node ids, or omit backendNodeId to target the document root.",
|
error.NodeNotFound => "NodeNotFound: the selector or backendNodeId matched nothing on the current page. Re-inspect the page (tree/interactiveElements) for fresh node ids, or omit backendNodeId to target the document root.",
|
||||||
error.FrameNotLoaded => "FrameNotLoaded: no page is loaded — call goto (or pass a url) first.",
|
error.FrameNotLoaded => "FrameNotLoaded: no page is loaded — call goto (or pass a url) first.",
|
||||||
|
error.NavigationTimeout => "NavigationTimeout: no response arrived before the timeout, so the page is empty. Other sessions may be holding every connection (see --http-max-concurrent); retry goto or close idle sessions.",
|
||||||
else => @errorName(err),
|
else => @errorName(err),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -2455,6 +2457,7 @@ fn performGoto(session: *lp.Session, registry: *NodeRegistry, url: [:0]const u8,
|
|||||||
// re-fetch frame, navigate might have changed it.
|
// re-fetch frame, navigate might have changed it.
|
||||||
const frame = page.frame() orelse return ToolError.NavigationFailed;
|
const frame = page.frame() orelse return ToolError.NavigationFailed;
|
||||||
if (frame._last_navigate_error != null) return ToolError.NavigationFailed;
|
if (frame._last_navigate_error != null) return ToolError.NavigationFailed;
|
||||||
|
if (result == .timeout and frame._parse_state == .pre) return ToolError.NavigationTimeout;
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2702,6 +2705,32 @@ test "tree and nodeDetails read the node's own frame" {
|
|||||||
try std.testing.expect(std.mem.indexOf(u8, details.text, "child-label") != null);
|
try std.testing.expect(std.mem.indexOf(u8, details.text, "child-label") != null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test "goto: a navigation stuck waiting for a connection is an error" {
|
||||||
|
var registry: NodeRegistry = .init(std.testing.allocator);
|
||||||
|
defer registry.deinit();
|
||||||
|
|
||||||
|
const network = &testing.test_app.network;
|
||||||
|
var held: std.ArrayList(*@import("../network/http.zig").Connection) = .empty;
|
||||||
|
defer held.deinit(std.testing.allocator);
|
||||||
|
defer for (held.items) |conn| network.releaseConnection(conn);
|
||||||
|
while (network.getConnection()) |conn| try held.append(std.testing.allocator, conn);
|
||||||
|
|
||||||
|
const session = testing.test_session;
|
||||||
|
defer if (session.primaryPage()) |page| page.close();
|
||||||
|
|
||||||
|
const aa = testing.arena_allocator;
|
||||||
|
const args = try std.json.parseFromSliceLeaky(std.json.Value, aa,
|
||||||
|
\\{"url":"http://localhost:9582/src/browser/tests/mcp_actions.html","timeout":300}
|
||||||
|
, .{});
|
||||||
|
try std.testing.expectError(error.NavigationTimeout, call(aa, session, ®istry, "goto", args, .{}));
|
||||||
|
|
||||||
|
for (held.items) |conn| network.releaseConnection(conn);
|
||||||
|
held.clearRetainingCapacity();
|
||||||
|
|
||||||
|
const r = try call(aa, session, ®istry, "goto", args, .{});
|
||||||
|
try std.testing.expectEqualStrings("Navigated successfully.", r.text);
|
||||||
|
}
|
||||||
|
|
||||||
test "parseValue: zero-filled optional backendNodeId treated as omitted" {
|
test "parseValue: zero-filled optional backendNodeId treated as omitted" {
|
||||||
var arena: std.heap.ArenaAllocator = .init(std.testing.allocator);
|
var arena: std.heap.ArenaAllocator = .init(std.testing.allocator);
|
||||||
defer arena.deinit();
|
defer arena.deinit();
|
||||||
|
|||||||
+1
-1
@@ -166,7 +166,7 @@ fn dispatchBrowserTool(
|
|||||||
error.FrameNotLoaded => .FrameNotLoaded,
|
error.FrameNotLoaded => .FrameNotLoaded,
|
||||||
error.NodeNotFound, error.InvalidParams => .InvalidParams,
|
error.NodeNotFound, error.InvalidParams => .InvalidParams,
|
||||||
error.Cancelled => .Cancelled,
|
error.Cancelled => .Cancelled,
|
||||||
error.Timeout => .Timeout,
|
error.Timeout, error.NavigationTimeout => .Timeout,
|
||||||
error.NavigationFailed, error.InternalError, error.OutOfMemory => .InternalError,
|
error.NavigationFailed, error.InternalError, error.OutOfMemory => .InternalError,
|
||||||
};
|
};
|
||||||
return server.sendError(id, code, browser_tools.errorMessage(err));
|
return server.sendError(id, code, browser_tools.errorMessage(err));
|
||||||
|
|||||||
@@ -802,7 +802,6 @@ pub fn _tick(self: *Client, timeout_ms: u32, mode: DrainMode) !bool {
|
|||||||
// we're about to tell our caller not to call us again without it
|
// we're about to tell our caller not to call us again without it
|
||||||
// doing some work (e.g. running tasks). Let's assert that we were
|
// doing some work (e.g. running tasks). Let's assert that we were
|
||||||
// right in doing that, else we'll likely introduce latency.
|
// right in doing that, else we'll likely introduce latency.
|
||||||
std.debug.assert(self.pending_queue.first == null);
|
|
||||||
std.debug.assert(self.delayed_queue.first == null);
|
std.debug.assert(self.delayed_queue.first == null);
|
||||||
std.debug.assert(self.dispatch_queue.first == null);
|
std.debug.assert(self.dispatch_queue.first == null);
|
||||||
std.debug.assert(self.ws_dispatch_queue.first == null);
|
std.debug.assert(self.ws_dispatch_queue.first == null);
|
||||||
|
|||||||
Reference in new issue
Block a user