Display UI to view app host logs in extension#17465
Open
JamesNK wants to merge 1 commit into
Open
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17465Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17465" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Aspire VS Code extension’s running AppHosts tree to surface the AppHost log file path (when provided by aspire ps JSON output) and adds a “View source” action to inspect the raw aspire ps data for an AppHost instance.
Changes:
- Add an “AppHost logs” tree item (when
logFilePathis present) that opens the log file, plus context menu actions to open/copy the path. - Add a “View source” context menu action for AppHost/workspace items that opens a virtual JSON document containing the raw
aspire psdata. - Remove PID child tree items (AppHost PID / CLI PID) from the tree.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| extension/src/views/AspireAppHostTreeProvider.ts | Removes PID child nodes; adds LogFileItem, new commands, and a TextDocumentContentProvider-backed “View source” virtual document. |
| extension/src/views/AppHostDataRepository.ts | Extends AppHostDisplayInfo to include optional logFilePath. |
| extension/src/loc/strings.ts | Adds localized label for the log-file tree item. |
| extension/src/extension.ts | Registers new commands for viewing AppHost source and opening/copying the log file path; removes PID copy command registration. |
| extension/package.nls.json | Updates localized command titles (replaces Copy ID with View source; adds log-file related commands). |
| extension/package.json | Adds command contributions and context menu wiring for “View source” and log file actions; removes PID menu wiring. |
- Add logFilePath to AppHostDisplayInfo from aspire ps JSON output - Show 'AppHost logs' tree item when log path is available - Click opens the log file in VS Code editor - Add 'Copy Path' context menu on log file item - Add 'View source' context menu on appHost to show raw JSON - Remove PID tree items (PID still shown in appHost description) Fixes #17460
JamesNK
force-pushed
the
jnewman/apphost-logs-ui
branch
from
529c038 to
db230ae
Compare
Contributor
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Contributor
|
❓ CLI E2E Tests unknown — 95 passed, 0 failed, 6 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26404556775 |
mitchdenny
reviewed
May 25, 2026
Member
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Two issues from review:
viewAppHostLogFilesilently swallows both invalid arguments and file-open errors.registerTextDocumentContentProviderin the constructor can leak across test instances.
| await vscode.window.showTextDocument(document, { preview: false }); | ||
| } catch { | ||
| vscode.window.showWarningMessage(logFileOpenFailed(filePath)); | ||
| } |
Member
There was a problem hiding this comment.
viewAppHostLogFile is a command system boundary — element can be anything when invoked from the command palette or another extension. Two problems with the current error handling:
- When given an invalid argument (e.g., an object, or a path-less click), the method silently returns with no user feedback. Consider showing a warning so misuse is visible rather than appearing to do nothing.
- The
catch {}discards the underlying error.vscode.workspace.openTextDocumentwill surface useful detail (file not found, permission denied, etc.). Includingerror instanceof Error ? error.message : String(error)inlogFileOpenFailedwould let users diagnose the failure without checking the extension log.
| this._dataSubscription = this._repository.onDidChangeData(() => { | ||
| this._onDidChangeTreeData.fire(); | ||
| }); | ||
| this._contentProviderRegistration = vscode.workspace.registerTextDocumentContentProvider('aspire-source', this); |
Member
There was a problem hiding this comment.
Registering the aspire-source TextDocumentContentProvider unconditionally in the constructor means each AspireAppHostTreeProvider instance creates a global registration for that scheme. In production there is only one instance, so this works, but in the unit tests many providers are constructed and not all paths call dispose(). VS Code keeps every leaked registration alive for the rest of the test run, and provideTextDocumentContent may then be dispatched to a stale provider whose _appHostSourceContents map does not contain the requested URI — returning "" and producing confusing test failures or flakiness if test ordering changes.
Two options:
- Lazy-register on first call to
viewAppHostSource(and dispose indispose()), so providers that never open a source document do not register. - Accept the registration as a constructor parameter so tests can pass a shared no-op or a per-test fresh one.
mitchdenny
approved these changes
May 25, 2026
Member
|
Tested this out manually locally and it all works fine. |
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
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.
Summary
aspire psnow includes the app host log file path in JSON output. This PR adds UI in the extension to surface it.extensions-log-path.mp4
I also removed the PID tree items. They don't seem like high value information. Instead, they're now accessible by right clicking the apphost and choosing "View source". This gives a complete JSON breakdown of all the apphost information.
Changes
logFilePathis present in theaspire psoutput, an "AppHost logs" item appears under the app host in the tree view. Clicking it opens the log file in VS Code.aspire psdata for that app host instance.Fixes #17460