-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Consolidate extension console logs and capture in Playwright tests #798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…e tests Add log capture for background service worker, offscreen document, and popup page console output during e2e tests. Logs are written to a timestamped file in <package>/logs/ and the path is attached to Playwright test results for easy access. Co-Authored-By: Claude <noreply@anthropic.com>
…ames Log files are now named with a run ID (timestamp of Playwright invocation) and test name to allow grouping logs from the same test run and easily identifying which test produced which logs. Format: <runId>-<testName>.log Co-Authored-By: Claude <noreply@anthropic.com>
Update yarn constraints to enforce ./logs in all workspace clean scripts, ensuring e2e test log files are cleaned up with other build artifacts. Co-Authored-By: Claude <noreply@anthropic.com>
Move console message forwarding functionality from the extension package to @metamask/kernel-browser-runtime as reusable utilities. This enables log capture from offscreen documents and other contexts via DuplexStream. - Create console-forwarding utilities with types and validators - Update extension to import from kernel-browser-runtime - Add console-forwarding exports to kernel-browser-runtime index Co-Authored-By: Claude <noreply@anthropic.com>
…ude script Implement a prelude script that wraps console methods BEFORE module code runs, enabling capture of early initialization logs that were previously lost. Uses chrome.runtime.sendMessage for immediate forwarding to background without waiting for stream setup. Changes: - Add console-forwarding-prelude.js to kernel-browser-runtime/src/static - Extend html-trusted-prelude Vite plugin to accept additional preludes - Configure extension to inject prelude before endoify.js - Update background.ts to handle console-forward-prelude messages - Remove stream-based console forwarding from offscreen.ts (now handled by prelude) - Configure eslint to ignore static/*.js files Co-Authored-By: Claude <noreply@anthropic.com>
…via prelude script" This reverts commit 20105ded24b211b08faab6d03d1efb6523c15d1f.
Enhance Playwright test logging to capture console output from all extension
contexts including vat iframes and kernel workers. Uses Chrome DevTools
Protocol (CDP) to access execution contexts that Playwright doesn't expose
directly through its API.
Changes:
- Add CDP-based console capture for iframe logs (vat iframes)
- Add worker.on('console') listener for kernel worker logs
- Add writeRawLog helper for CDP events without ConsoleMessage objects
- Track CDP sessions for proper cleanup
- Add CDP type definitions for Runtime domain events
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…service worker Add source field to ConsoleForwardMessage to identify log origins (offscreen, kernel-worker, vat-*). Forward console logs from kernel worker and vat iframes through offscreen to background, enabling centralized logging for production and simplified Playwright capture. - Add source field to ConsoleForwardMessage type - Add source parameter to setupConsoleForwarding() - Extract stringifyConsoleArg() as public utility - Forward kernel worker logs via stream to background - Forward vat iframe logs via postMessage to offscreen - Add unit tests for console-forwarding module Co-Authored-By: Claude <noreply@anthropic.com>
…me logs Move setupIframeConsoleForwarding to console-forwarding.ts as setupPostMessageConsoleForwarding and use the standard ConsoleForwardMessage format. This allows offscreen to validate messages using isConsoleForwardMessage and simplifies the relay logic. - Add setupPostMessageConsoleForwarding() for iframe console forwarding - Use standard ConsoleForwardMessage format (jsonrpc + params) via postMessage - Simplify offscreen message handler to use isConsoleForwardMessage - Add unit tests for setupPostMessageConsoleForwarding Co-Authored-By: Claude <noreply@anthropic.com>
Add attachLogs() function that reads the log file and attaches it to test results. The extension helper now wraps browserContext.close() to auto-attach logs before closing, so tests don't need to be modified. This fixes the issue where logs were written to packages/extension/logs/ but not attached to test-results for viewing in the Playwright HTML report. Co-Authored-By: Claude <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
| } | ||
| // Objects, arrays, null, undefined, functions, symbols, etc. | ||
| return JSON.stringify(arg); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stringifyConsoleArg returns undefined instead of string
Medium Severity
The stringifyConsoleArg function declares return type string but returns undefined when given undefined input. JSON.stringify(undefined) returns the value undefined, not a string. This causes the args array (typed as string[]) to contain undefined values at runtime. When serialized for transmission, undefined in arrays becomes null, so logs containing undefined will incorrectly display as null, altering the meaning of debug output.
Additional Locations (1)
| // Only set up CDP for pages that might have iframes (offscreen document) | ||
| if (!page.url().includes('offscreen.html')) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CDP setup checks URL before page navigation completes
Low Severity
The setupCdpForIframeConsoleLogs function checks page.url() synchronously at line 222, but the comment on line 161 acknowledges that "Pages may start at about:blank". The console handler correctly checks URL inside its callback, but the CDP setup returns early if the page hasn't navigated yet, potentially causing iframe console logs to never be captured via CDP.
| typeof value === 'object' && | ||
| value !== null && | ||
| 'method' in value && | ||
| (value as { method: unknown }).method === 'console-forward'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type guard validates method but not params structure
Low Severity
The isConsoleForwardMessage type guard only validates that method === 'console-forward', but asserts the value is a full ConsoleForwardMessage with properly structured params. When handleConsoleForwardMessage destructures message.params to access source, method, and args, a malformed message like {method: 'console-forward'} would pass the guard but crash with a TypeError. Since offscreen.ts accepts messages via window.addEventListener('message') from any iframe, a malformed postMessage could crash the background script.
Additional Locations (1)
| }); | ||
|
|
||
| harden(globalThis.console); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated console wrapping logic in two functions
Medium Severity
setupConsoleForwarding (lines 60-90) and setupPostMessageConsoleForwarding (lines 115-139) share nearly identical logic: copying original console, iterating the same methods array, wrapping each method, creating identical ConsoleForwardMessage structures, mapping args with stringifyConsoleArg, and calling harden(). Only the forwarding mechanism differs. A shared helper function could eliminate this duplication.
| }); | ||
|
|
||
| // Track CDP sessions for cleanup | ||
| const cdpSessions: CDPSession[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||


Summary
This PR consolidates all console logs from the extension's various contexts (background, offscreen, kernel worker, vat iframes) into the background service worker, and captures them in
packages/*/logsduring Playwright e2e tests for debugging.Previously, console logs from different extension contexts were difficult to capture in Playwright tests because:
Now, all console logs flow through the background service worker using a standardized
ConsoleForwardMessageformat, making capture straightforward.Changes
Console forwarding infrastructure (
kernel-browser-runtime):sourcefield toConsoleForwardMessageto identify log originssetupConsoleForwarding()for stream-based forwarding (offscreen, kernel-worker)setupPostMessageConsoleForwarding()for iframe-based forwarding (vat iframes)stringifyConsoleArg()utility for consistent argument serializationExtension integration:
[offscreen],[kernel-worker],[vat-v1]Playwright test capture (
repo-tools):Dependencies: Bump Playwright to latest version
Testing
Unit tests cover the console-forwarding module including message type guards, argument stringification, stream-based forwarding, and postMessage-based forwarding. The e2e tests exercise the full flow by running the extension and capturing logs.
🤖 Generated with Claude Code
Note
Medium Risk
Medium risk because it changes runtime behavior in multiple extension contexts by monkey-patching
consoleand routing new JSON-RPC messages through existing streams; mistakes could affect logging/perf or message handling.Overview
Adds a standardized JSON-RPC
console-forwardmessage flow to consolidate extension logs (vat iframes → offscreen → background; kernel worker/offscreen → background) and replay them in the background console for easier debugging.Introduces
kernel-browser-runtimeutilities (setupConsoleForwarding,setupPostMessageConsoleForwarding,handleConsoleForwardMessage,isConsoleForwardMessage,stringifyConsoleArg) with unit tests, and wires them intobackground.ts,offscreen.ts,kernel-worker.ts, andvat/iframe.ts.Enhances Playwright extension test utilities to persist per-test console logs to
./logsand auto-attach them to test reports; updates workspacecleanscripts to remove./logsand bumps Playwright deps to^1.57.0.Written by Cursor Bugbot for commit fbaaaef. This will update automatically on new commits. Configure here.