Skip to content

Fix browser debug targets, empty webRoot, and resource stop ordering - #19145

Draft
Adam Ratzman (adamint) wants to merge 2 commits into
microsoft:mainfrom
adamint:adamint/vscode-resource-debug-fixes
Draft

Fix browser debug targets, empty webRoot, and resource stop ordering#19145
Adam Ratzman (adamint) wants to merge 2 commits into
microsoft:mainfrom
adamint:adamint/vscode-resource-debug-fixes

Conversation

@adamint

Copy link
Copy Markdown
Member

Description

Three user-visible fixes to VS Code extension debugging, split out of #19133 so they are not blocked on the E2E harness in that PR.

1. Browser resources fail with an opaque adapter error. browser.ts built the debug type as pwa-${browser}, so any value other than chrome/msedge produced Configured debug type '<x>' is not supported from VS Code, with no indication of which resource or which value caused it. Browser debug targets now resolve through a ReadonlyMap allowlist and an unsupported value throws a named error identifying the resource and the accepted values.

A Map is used rather than an object literal so that inputs such as toString, constructor and __proto__ cannot reach Object.prototype and assign a non-string value to debugConfiguration.type. The hosting API accepts arbitrary strings here, so the lookup has to be safe for any input.

2. Empty web_root produced a broken source map root. An empty string was assigned straight to debugConfiguration.webRoot, which js-debug resolves against the workspace and then fails to map sources. Empty and whitespace-only values are now left unset.

3. stopDebugging() stopped the AppHost before its resources. Stopping the AppHost first tears down the orchestrator that owns the resource processes, so resource debug sessions could be left orphaned or report spurious termination errors. Resource sessions are now stopped first (in parallel, excluding the AppHost session), then the AppHost, then the parent session. A resource stop failure is rethrown after the remaining sessions have been stopped, so one failure cannot strand the rest.

Related

Validation

From extension/:

yarn run compile-tests   Done in 3.44s
yarn run lint            Done in 2.83s
yarn run compile         webpack succeeded
yarn run unit-test       1473 passing, 4 pending, 0 failing

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
  • Did you add public API?
    • No
  • Does the change make any security assumptions or guarantees?
    • No

Copilot AI balanced review requested due to automatic review settings August 7, 2026 18:12
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19145

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19145"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes VS Code debugging configuration and shutdown behavior.

Changes:

  • Validates browser adapters and omits empty web roots.
  • Stops resource sessions before AppHost and parent sessions.
  • Localizes Azure Functions debugger labels and adds tests.
Show a summary per file
File Description
extension/src/test/browserDebugger.test.ts Tests browser configuration and validation.
extension/src/test/azureFunctionsDebugger.test.ts Tests Azure Functions debugger metadata.
extension/src/test/aspireDebugSession.test.ts Tests debugger shutdown ordering.
extension/src/loc/strings.ts Adds localized debugger strings.
extension/src/debugger/languages/browser.ts Validates browsers and handles web roots.
extension/src/debugger/languages/azureFunctions.ts Localizes session names.
extension/src/debugger/AspireDebugSession.ts Reorders session shutdown.
extension/package.nls.json Registers localization strings.
extension/loc/xlf/aspire-vscode.xlf Updates generated localization data.

Review details

  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread extension/src/debugger/AspireDebugSession.ts Outdated
Comment thread extension/src/debugger/languages/browser.ts Outdated
@adamint
Adam Ratzman (adamint) marked this pull request as draft August 7, 2026 18:16
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Copilot AI review requested due to automatic review settings August 7, 2026 19:46
Adam Ratzman and others added 2 commits August 7, 2026 15:49
Four fixes to the VS Code resource debugger, extracted from microsoft#19133 so they are
not gated on an E2E harness.

`WithBrowserDebugger(browser)` takes an arbitrary string and `browser.ts`
forwarded it as `pwa-<value>`. js-debug only contributes `pwa-chrome` and
`pwa-msedge`, so anything else failed inside VS Code with "Configured debug type
is not supported" after the session had already started - no resource name, no
indication of which browsers do work. Unknown values now fail up front with the
supported list. The allowlist is a `Map` rather than an object literal because
the key is caller-supplied: a literal inherits `Object.prototype`, so a resource
named `toString` or `__proto__` would resolve to an inherited member and assign a
function to `debugConfiguration.type`.

The hosting side sends `web_root: ""` when a browser resource has no web root,
and js-debug treats an empty `webRoot` as a real path when it resolves source
maps. Only forward a value the AppHost actually configured.

`stopDebugging()` stopped the AppHost first and left resource debug sessions to
`dispose()`. A resource running under a debugger can hold the AppHost shutdown
open until its own session exits, so the AppHost stop waited on a process whose
debugger had not been told to stop yet. Resource sessions now stop first, and a
failure there is rethrown only after the AppHost and the synthetic Aspire parent
have been stopped, so one bad adapter cannot strand the rest.

The Azure Functions debug-session names were plain template strings, so they were
never extracted for localization despite being user-visible session names. They
use `vscode.l10n.t` now, like the adjacent browser labels.

Adds `browserDebugger.test.ts`, extends the `stopDebugging` ordering test, and
adds a metadata suite to `azureFunctionsDebugger.test.ts` that pins the localized
names. 1473 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`Promise.all` settles on the first rejection, so a resource adapter that failed
to stop released `stopDebugging()` while the remaining resource stops were still
in flight and the AppHost stop began underneath them. That is the ordering this
change exists to enforce, broken on precisely the path where a resource is most
likely to be orphaned. `Promise.allSettled` now waits for every adapter; a single
failure is rethrown unchanged after the AppHost and the synthetic Aspire parent
have been stopped, and multiple failures are rethrown as an `AggregateError` so
no reason is discarded the way `Promise.all` discarded them.

`if (launchConfig.web_root)` let a whitespace-only value through because `"   "`
is truthy, so js-debug still received an invalid source-map root - the same
outcome the empty-string guard was added to prevent. The trimmed value is now
both the thing tested and the thing forwarded, so what was validated is what
js-debug gets.

`stopDebugging waits for every resource stop to settle before stopping the
AppHost` fails against the old `Promise.all` with the AppHost and parent stops
recorded ahead of `slow-resource-session-settled`. The blank web root cases
(`""`, `"   "`, `"\t"`, `"\n"`, `" \t\r\n "`) plus the trimmed-forwarding case
give 5 failures against the untrimmed guard. 1480 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

extension/src/debugger/AspireDebugSession.ts:136

  • This lifecycle fix is covered only with a stubbed vscode.debug.stopDebugging, so it cannot detect the real VS Code termination/disposal race or verify that the resource process is gone before AppHost teardown. The linked E2E coverage in #19133 is still an open draft, meaning this fix can merge without a regression test for the orphaned-process symptom. Add equivalent coverage under extension/src/test-e2e/ that starts a resource, stops the Aspire session, and verifies the resource process exits.
    const resourceStopResults = await Promise.allSettled(resourceDebugSessions.map(session => session.stopSession()));

extension/src/debugger/AspireDebugSession.ts:161

  • An AppHost or parent stop rejection exits this try/finally before the resource failures below are examined. When teardown failures coincide, the resource failure promised by this method is silently lost (and a parent failure also masks an AppHost failure). Capture each stop error, then preserve a single reason or throw one aggregate after every stop has been attempted.
    try {
      await this._appHostDebugSession?.stopSession();
    }
    finally {
      await this.stopParentDebugSessionOnce();
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@adamint
Adam Ratzman (adamint) force-pushed the adamint/vscode-resource-debug-fixes branch from 70f1562 to 6d2f67a Compare August 7, 2026 19:56
Copilot AI review requested due to automatic review settings August 7, 2026 19:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

extension/src/debugger/AspireDebugSession.ts:161

  • When an AppHost stop rejects, control exits after the finally, so the resource failures collected above are never rethrown; a parent-stop rejection can similarly replace both earlier failures. This loses the shutdown diagnostics this change promises whenever failures occur at more than one level. Capture AppHost and parent failures too, finish every stop, then throw the sole failure or an aggregate of all failures.
    try {
      await this._appHostDebugSession?.stopSession();
    }
    finally {
      await this.stopParentDebugSessionOnce();
    }
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants