WIP: Add tracked browser logs and network capture#16310
Draft
WIP: Add tracked browser logs and network capture#16310
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16310Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16310" |
Add a child browser logs resource that can launch tracked Chromium sessions, stream console and network request events, and expose the builder through polyglot export.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
force-pushed
the
spike/browser-console-logs-poc
branch
from
5c31105 to
eab2c02
Compare
davidfowl
changed the title
Add typed CDP protocol parsing, preserve connection failure reasons in resource logs, and strengthen reconnect handling for tracked browser sessions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
reviewed
Apr 19, 2026
Member
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
5 issues found (1 security, 2 bugs, 1 race condition, 1 formatting).
| "--no-default-browser-check", | ||
| "--new-window", | ||
| "--allow-insecure-localhost", | ||
| "--ignore-certificate-errors", |
Member
There was a problem hiding this comment.
--allow-insecure-localhost on line 1035 already covers the local development use case. Adding --ignore-certificate-errors additionally disables certificate validation for all sites in the tracked browser, which could expose users to MITM attacks if they navigate away from localhost. Consider removing this flag.
|
|
||
| foreach (var (_, resourceState) in _resourceStates) | ||
| { | ||
| resourceState.Lock.Dispose(); |
Member
There was a problem hiding this comment.
DisposeAsync stops all sessions (line 192-195) then disposes all Lock instances here. However, StartCompletionObserver (line 153) launches a fire-and-forget ObserveCompletionAsync that calls HandleSessionCompletedAsync, which acquires this Lock (line 211). After session.StopAsync() returns, the observer's continuation may still be pending, causing an ObjectDisposedException here. While the ObserveCompletionAsync catch block (line 1246) swallows it, this race could mask real errors.
Fix indentation of <remarks> block in BrowserLogsBuilderExtensions.cs to match surrounding XML doc tags. Thread TimeProvider through RunningSession and its factory so timeout calculations in WaitForBrowserEndpointAsync and TryReconnectAsync use the injected provider instead of TimeProvider.System/DateTime.UtcNow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract the running-session lifecycle, CDP websocket transport, and event/diagnostic logging into focused files so the browser connection layers can be tested in isolation. Also add targeted comments describing the browser and CDP quirks these layers handle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the global certificate-bypass flag from tracked browser launches and make session-manager disposal wait for completion observers without publishing shutdown updates through torn-down services. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24642063830 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
WithBrowserLogs(...)toAspire.Hostingso browser-capable resources can attach a child*-browser-logsresource that opens tracked Chromium sessions and streams browser activity back into the dashboard log stream.This draft includes:
Network.*events;AspireExportsupport sowithBrowserLogs(...)is emitted in the polyglot AppHost generators while the child resource remains excluded from manifest publishing;Browser connection architecture
Ignoring the Aspire-specific resource wiring, the browser side is a small Chromium DevTools Protocol client:
--remote-debugging-port=0.DevToolsActivePortfile to discover the actual debug endpoint.Runtime,Log,Page, andNetworkdomains.Internally those responsibilities are now split across
BrowserLogsSessionManager,BrowserLogsRunningSession,ChromeDevToolsConnection,BrowserLogsProtocol,BrowserEventLogger, andBrowserConnectionDiagnosticsLogger.Validation
Validation included targeted
Aspire.Hosting.Testscoverage for resource wiring, session behavior, protocol parsing, request formatting, and diagnostics logging, focused ATS/codegen coverage for the exported capability, and liveplayground/BrowserTelemetryruns confirming real[network.*]and browser log lines inweb-browser-logsviaaspire resource web-browser-logs open-tracked-browser.Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: