fix: Prevent CDP timeout on empty URL tabs (#102)
When connecting to a browser via CDP, particularly on Android, tabs with an empty URL can cause Playwright commands to hang indefinitely. This leads to a timeout in agent-browser. This commit fixes the issue by filtering out any pages that have an empty `page.url()` during the CDP connection process. This prevents agent-browser from attempting to interact with these problematic tabs, resolving the timeout while preserving normal pages. Added a unit test to verify that pages with empty URLs are correctly ignored. Also increased the timeout for a flaky screencast test to improve test suite stability. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: sheing-google <231310897+sheing-google@users.noreply.github.com>
This commit is contained in:
co-authored by
google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
sheing-google
parent
e196ed3e35
commit
7aad47d3bd
+32
-2
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
|
||||
import { BrowserManager } from './browser.js';
|
||||
import { chromium } from 'playwright-core';
|
||||
|
||||
describe('BrowserManager', () => {
|
||||
let browser: BrowserManager;
|
||||
@@ -390,6 +391,35 @@ describe('BrowserManager', () => {
|
||||
const cdp2 = await browser.getCDPSession();
|
||||
expect(cdp1).toBe(cdp2);
|
||||
});
|
||||
|
||||
it('should filter out pages with empty URLs during CDP connection', async () => {
|
||||
const mockBrowser = {
|
||||
contexts: () => [
|
||||
{
|
||||
pages: () => [
|
||||
{ url: () => 'http://example.com', on: vi.fn() },
|
||||
{ url: () => '', on: vi.fn() }, // This page should be filtered out
|
||||
{ url: () => 'http://anothersite.com', on: vi.fn() },
|
||||
],
|
||||
on: vi.fn(),
|
||||
},
|
||||
],
|
||||
close: vi.fn(),
|
||||
};
|
||||
const spy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any);
|
||||
|
||||
const cdpBrowser = new BrowserManager();
|
||||
await cdpBrowser.launch({ cdpPort: 9222 });
|
||||
|
||||
// Should have 2 pages, not 3
|
||||
expect(cdpBrowser.getPages().length).toBe(2);
|
||||
|
||||
// Verify that the empty URL page is not in the list
|
||||
const urls = cdpBrowser.getPages().map((p) => p.url());
|
||||
expect(urls).not.toContain('');
|
||||
expect(urls).toContain('http://example.com');
|
||||
spy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('screencast', () => {
|
||||
@@ -405,7 +435,7 @@ describe('BrowserManager', () => {
|
||||
expect(browser.isScreencasting()).toBe(true);
|
||||
|
||||
// Wait a bit for at least one frame
|
||||
await new Promise((resolve) => setTimeout(resolve, 200));
|
||||
await new Promise((resolve) => setTimeout(resolve, 1000));
|
||||
|
||||
await browser.stopScreencast();
|
||||
expect(browser.isScreencasting()).toBe(false);
|
||||
|
||||
+5
-1
@@ -738,7 +738,11 @@ export class BrowserManager {
|
||||
throw new Error('No browser context found. Make sure the app has an open window.');
|
||||
}
|
||||
|
||||
const allPages = contexts.flatMap((context) => context.pages());
|
||||
// Filter out pages with empty URLs, which can cause Playwright to hang
|
||||
const allPages = contexts
|
||||
.flatMap((context) => context.pages())
|
||||
.filter((page) => page.url());
|
||||
|
||||
if (allPages.length === 0) {
|
||||
throw new Error('No page found. Make sure the app has loaded content.');
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user