add test to find out how to remove sleep after extension connect
This commit is contained in:
@@ -330,11 +330,25 @@ This ensures the page has a **valid URL and frame structure** before user code s
|
||||
### Key Insights
|
||||
|
||||
- `context.on('page')` and `context.waitForEvent('page')` fire on the **same event**
|
||||
- **`waitForEvent('page')` only waits for NEW pages** - it will NOT resolve with existing pages
|
||||
- To handle existing pages, check `context.pages()` first before waiting
|
||||
- The 'page' event fires AFTER page initialization, not immediately on attach
|
||||
- `Runtime.executionContextCreated` is NOT required for the 'page' event
|
||||
- The 'page' event guarantees the page has a valid URL and frame tree
|
||||
- `page.evaluate()` may still need to wait for execution context after 'page' fires
|
||||
|
||||
### Handling Existing vs New Pages
|
||||
|
||||
```typescript
|
||||
// waitForEvent only catches NEW pages, not existing ones
|
||||
const existingPages = context.pages();
|
||||
if (existingPages.length > 0) {
|
||||
return existingPages[0]; // use existing
|
||||
}
|
||||
// Only wait if no pages exist yet
|
||||
const newPage = await context.waitForEvent('page');
|
||||
```
|
||||
|
||||
### Timing Implications for CDP Relay
|
||||
|
||||
For `context.on('page')` to fire, the relay must:
|
||||
@@ -344,3 +358,41 @@ For `context.on('page')` to fire, the relay must:
|
||||
3. Either:
|
||||
- Provide a non-empty URL (not `:`) in the frame tree
|
||||
- Or send `Page.frameNavigated` event after navigation
|
||||
|
||||
## Empty URL Detection
|
||||
|
||||
Empty URLs in `Target.attachedToTarget` cause Playwright to create broken pages that
|
||||
never recover. Both the extension and relay log errors when this happens:
|
||||
|
||||
### Extension logging
|
||||
|
||||
```typescript
|
||||
// In attachTab(), after Target.getTargetInfo
|
||||
if (!targetInfo.url || targetInfo.url === '' || targetInfo.url === ':') {
|
||||
logger.error('WARNING: Target.attachedToTarget will be sent with empty URL!')
|
||||
}
|
||||
```
|
||||
|
||||
### Relay logging
|
||||
|
||||
The relay logs `logger.error` warnings when:
|
||||
- `Target.attachedToTarget` is sent/received with empty URL
|
||||
- `Target.targetCreated` is sent with empty URL
|
||||
|
||||
### Why empty URLs break Playwright
|
||||
|
||||
If `Target.attachedToTarget` is sent with empty URL:
|
||||
- Playwright creates a broken page
|
||||
- `_firstNonInitialNavigationCommittedPromise` may never resolve
|
||||
- `page.evaluate()` hangs forever
|
||||
- **No recovery possible**
|
||||
|
||||
### Debugging empty URL issues
|
||||
|
||||
Check the logs at `~/.config/opencode/playwriter.log` for:
|
||||
```
|
||||
WARNING: Target.attachedToTarget sent with empty URL
|
||||
WARNING: Target.attachedToTarget received with empty URL
|
||||
```
|
||||
|
||||
These indicate timing issues where the page hasn't fully loaded when attached.
|
||||
|
||||
@@ -1260,6 +1260,45 @@ describe('MCP Server Tests', () => {
|
||||
await page.close()
|
||||
}, 30000)
|
||||
|
||||
it('should be usable after toggle with waitForEvent', async () => {
|
||||
// This test demonstrates the proper way to wait for a page after toggle.
|
||||
// Instead of arbitrary sleep(), use context.waitForEvent('page').
|
||||
// See page-ready-investigation.md for details on why this is needed.
|
||||
|
||||
const _browserContext = getBrowserContext()
|
||||
const serviceWorker = await getExtensionServiceWorker(_browserContext)
|
||||
const browser = await chromium.connectOverCDP(getCdpUrl({ port: TEST_PORT }))
|
||||
const context = browser.contexts()[0]
|
||||
|
||||
const page = await _browserContext.newPage()
|
||||
await page.goto('https://example.com', { waitUntil: 'domcontentloaded' })
|
||||
await page.bringToFront()
|
||||
|
||||
// Set up page listener BEFORE toggle - this is the key pattern
|
||||
const pagePromise = context.waitForEvent('page', {
|
||||
predicate: p => p.url().includes('example.com'),
|
||||
timeout: 5000
|
||||
})
|
||||
|
||||
// Toggle extension - no sleep needed after!
|
||||
await serviceWorker.evaluate(async () => {
|
||||
await globalThis.toggleExtensionForActiveTab()
|
||||
})
|
||||
|
||||
// Wait for page event - this resolves when Playwright has fully processed the page
|
||||
const targetPage = await pagePromise
|
||||
|
||||
// Page is now guaranteed to be ready
|
||||
expect(targetPage.url()).toContain('example.com')
|
||||
|
||||
// evaluate() works immediately
|
||||
const result = await targetPage.evaluate(() => window.location.href)
|
||||
expect(result).toContain('example.com')
|
||||
|
||||
await browser.close()
|
||||
await page.close()
|
||||
}, 30000)
|
||||
|
||||
it('should maintain correct page.url() with iframe-heavy pages', async () => {
|
||||
const browserContext = getBrowserContext()
|
||||
const serviceWorker = await getExtensionServiceWorker(browserContext)
|
||||
|
||||
Reference in New Issue
Block a user