Minimal JavaScript debugging support modeled after Python implementation#15067
Closed
adamint wants to merge 2 commits intomicrosoft:revert/14686from
Closed
Minimal JavaScript debugging support modeled after Python implementation#15067adamint wants to merge 2 commits intomicrosoft:revert/14686from
adamint wants to merge 2 commits intomicrosoft:revert/14686from
Conversation
Add Node.js debugging support with minimal changes: - NodeLaunchConfiguration extending ExecutableLaunchConfiguration - Internal WithVSCodeDebugging methods wired into AddNodeApp/CreateDefaultJavaScriptAppBuilder - Extension: node debugger using built-in js-debug adapter - No new public APIs or extensibility system - Tests updated for additional CommandLineArgsCallbackAnnotation from WithDebugSupport
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15067Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15067" |
Contributor
There was a problem hiding this comment.
Pull request overview
Reintroduces JavaScript/Node.js debugging support with a simplified “launch-configuration-only” model (similar to the Python flow), moving launch configuration types into the DCP model and updating the VS Code extension to generate debug configurations without apphost-provided debugger_properties.
Changes:
- Simplifies debug support plumbing:
SupportsDebuggingAnnotationnow provides a mode-only launch config annotator, and launch configuration types move underAspire.Hosting.Dcp.Model. - Re-adds Node debugging wiring in
Aspire.Hosting.JavaScriptand introduces a new VS Code node debugger extension (node.ts). - Updates tests/playground/extension capability signaling to match the simplified model (removing browser/debugger-properties infrastructure).
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/ExecutableResourceBuilderExtensionTests.cs | Updates tests for new launch config types/signatures. |
| tests/Aspire.Hosting.Tests/DebuggerPropertiesAnnotationTests.cs | Removes tests for deleted debugger-properties infrastructure. |
| tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs | Adjusts DCP executor tests for new launch config flow. |
| tests/Aspire.Hosting.Python.Tests/AddPythonAppTests.cs | Removes Python debugger-properties tests; minor cleanup. |
| tests/Aspire.Hosting.JavaScript.Tests/AddViteAppWithPnpmTests.cs | Uses .First() due to extra args annotations. |
| tests/Aspire.Hosting.JavaScript.Tests/AddViteAppTests.cs | Uses .First() due to extra args annotations. |
| tests/Aspire.Hosting.JavaScript.Tests/AddNodeAppTests.cs | Removes JS debug tests tied to removed APIs. |
| src/Shared/KnownConfigNames.cs | Removes ASPIRE_EXTENSION_WORKSPACE_ROOT constant. |
| src/Aspire.Hosting/Utils/ExtensionUtils.cs | Reworks debug-session config parsing into ExtensionUtils. |
| src/Aspire.Hosting/SupportsDebuggingAnnotation.cs | Simplifies annotator signature; removes producer options class. |
| src/Aspire.Hosting/ResourceBuilderExtensions.cs | Adds/changes WithDebugSupport surface and args hook. |
| src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs | Switches project debugging hookup to WithDebugSupport. |
| src/Aspire.Hosting/ProjectLaunchConfiguration.cs | Deletes old hosting-side project launch/debugger types. |
| src/Aspire.Hosting/ExecutableLaunchConfiguration.cs | Deletes old hosting-side launch/debugger property types. |
| src/Aspire.Hosting/ExecutableDebuggerPropertiesAnnotation.cs | Deletes debugger-properties annotation types. |
| src/Aspire.Hosting/DistributedApplicationBuilder.cs | Removes IBackchannelLoggerProvider registration. |
| src/Aspire.Hosting/Dcp/Model/RunSessionInfo.cs | Adds DCP model RunSessionInfo. |
| src/Aspire.Hosting/Dcp/Model/ModelCommon.cs | Removes ignore-null serialization behavior for annotations. |
| src/Aspire.Hosting/Dcp/Model/ExecutableLaunchConfiguration.cs | Adds DCP model launch configuration types. |
| src/Aspire.Hosting/Dcp/DcpExecutor.cs | Moves launch config annotation earlier; simplifies IDE execution handling. |
| src/Aspire.Hosting/CompatibilitySuppressions.xml | Removes suppression related to debug support API signature. |
| src/Aspire.Hosting/Backchannel/BackchannelLoggerProvider.cs | Removes IBackchannelLoggerProvider interface. |
| src/Aspire.Hosting/Aspire.Hosting.csproj | Removes shared debug support include + InternalsVisibleTo entries. |
| src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs | Reworks Python debugging to launch-config-only model. |
| src/Aspire.Hosting.Python/PythonAppLaunchConfiguration.cs | Simplifies Python launch configuration type. |
| src/Aspire.Hosting.Python/Aspire.Hosting.Python.csproj | Compiles DCP model launch config source into Python assembly. |
| src/Aspire.Hosting.JavaScript/NodeLaunchConfiguration.cs | Simplifies Node launch configuration type. |
| src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs | Re-adds minimal JS debugging wiring via WithDebugSupport. |
| src/Aspire.Hosting.JavaScript/BrowserLaunchConfiguration.cs | Removes browser debugging resource/config types. |
| src/Aspire.Hosting.JavaScript/Aspire.Hosting.JavaScript.csproj | Compiles DCP model launch config source into JS assembly. |
| src/Aspire.Cli/Utils/ExtensionHelper.cs | Removes browser capability constant. |
| src/Aspire.Cli/Program.cs | Simplifies exception handling around extension connection loss. |
| src/Aspire.Cli/Interaction/ExtensionInteractionService.cs | Simplifies error reporting path to extension backchannel. |
| src/Aspire.Cli/Commands/RunCommand.cs | Removes a connection-lost swallow path in run. |
| playground/python/Python.AppHost/Program.cs | Uses .WithDebugging() for python executable sample. |
| playground/Stress/Stress.AppHost/Program.cs | Removes pragma + sample project addition. |
| playground/AzureFunctionsEndToEnd/AzureFunctionsEndToEnd.AppHost/Program.cs | Removes pragma warning suppression. |
| playground/AspireWithJavaScript/AspireJavaScript.React/webpack.config.js | Removes explicit devtool: source-map. |
| playground/AspireWithJavaScript/AspireJavaScript.NodeApp/app.js | Changes 404 handler route matching. |
| playground/AspireWithJavaScript/AspireJavaScript.MinimalApi/Properties/launchSettings.json | Adds launchUrl: swagger. |
| playground/AspireWithJavaScript/AspireJavaScript.MinimalApi/AppHost.cs | Adds Swagger/OpenAPI wiring in sample. |
| playground/AspireWithJavaScript/AspireJavaScript.AppHost/AspireJavaScript.AppHost.csproj | Removes NoWarn for removed experimental browser API. |
| extension/src/utils/AspireTerminalProvider.ts | Removes IDE/workspace-root env vars and browser capability. |
| extension/src/server/rpcClient.ts | Removes stopCli request swallow. |
| extension/src/debugger/languages/python.ts | Removes deprecated flag usage (debug config now extension-driven). |
| extension/src/debugger/languages/node.ts | Adds node debugger extension implementation. |
| extension/src/debugger/languages/dotnet.ts | Removes deprecated flag usage. |
| extension/src/debugger/debuggerExtensions.ts | Removes debugger_properties merge path; requires per-language extensions. |
| extension/src/dcp/types.ts | Removes debugger_properties; adds node launch config type. |
| extension/src/dcp/AspireDcpServer.ts | Requires a debugger extension per launch config type. |
| extension/src/capabilities.ts | Drops browser capability; always reports node capability. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
967
to
+988
| [Experimental("ASPIREEXTENSION001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| internal static IResourceBuilder<T> WithBrowserDebugger<T>( | ||
| this IResourceBuilder<T> builder, | ||
| string browser, | ||
| Action<VSCodeBrowserDebuggerProperties>? configureDebuggerProperties) | ||
| internal static IResourceBuilder<T> WithVSCodeDebugging<T>(this IResourceBuilder<T> builder) | ||
| where T : JavaScriptAppResource | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); | ||
|
|
||
| // Validate that the extension supports browser debugging if we're running in an extension context | ||
| ValidateBrowserCapability(builder); | ||
|
|
||
| var parentResource = builder.Resource; | ||
| var debuggerResourceName = $"{parentResource.Name}-browser"; | ||
| // Get package manager info for runtime executable | ||
| var packageManager = "npm"; | ||
|
|
||
| // Create a placeholder debugger resource - the URL will be resolved when the callback is invoked | ||
| var debuggerResource = new VSCodeBrowserDebuggerResource( | ||
| debuggerResourceName, | ||
| browser, | ||
| parentResource.WorkingDirectory, | ||
| parentResource.WorkingDirectory, | ||
| "placeholder", // URL will be resolved in the callback after endpoints are allocated | ||
| configureDebuggerProperties); | ||
|
|
||
| // Find the parent's HTTP/HTTPS endpoint | ||
| EndpointAnnotation? endpointAnnotation = null; | ||
| if (parentResource.TryGetAnnotationsOfType<EndpointAnnotation>(out var endpoints)) | ||
| if (builder.Resource.TryGetLastAnnotation<JavaScriptPackageManagerAnnotation>(out var pmAnnotation)) | ||
| { | ||
| endpointAnnotation = endpoints.FirstOrDefault(e => e.UriScheme == "https") | ||
| ?? endpoints.FirstOrDefault(e => e.UriScheme == "http"); | ||
| packageManager = pmAnnotation.ExecutableName; | ||
| } | ||
|
|
||
| if (endpointAnnotation is null) | ||
| { | ||
| throw new InvalidOperationException($"Resource '{parentResource.Name}' does not have an HTTP or HTTPS endpoint. Browser debugging requires an endpoint to navigate to."); | ||
| } | ||
|
|
||
| var endpointReference = parentResource.GetEndpoint(endpointAnnotation.Name); | ||
|
|
||
| builder.ApplicationBuilder.AddResource(debuggerResource) | ||
| .WithParentRelationship(parentResource) | ||
| .WaitFor(builder) | ||
| .ExcludeFromManifest() | ||
| .WithDebugSupport( | ||
| options => | ||
| { | ||
| // The callback is invoked after endpoints are allocated, so we can access the URL directly | ||
| debuggerResource.DebuggerProperties.Url = endpointReference.Url; | ||
|
|
||
| return new BrowserLaunchConfiguration | ||
| { | ||
| Mode = options.Mode, | ||
| DebuggerProperties = debuggerResource.DebuggerProperties | ||
| }; | ||
| }, | ||
| "browser"); | ||
|
|
||
| return builder; | ||
| return builder.WithDebugSupport( | ||
| mode => new NodeLaunchConfiguration | ||
| { | ||
| ScriptPath = string.Empty, | ||
| Mode = mode, | ||
| RuntimeExecutable = packageManager | ||
| }, | ||
| "node"); |
There was a problem hiding this comment.
WithVSCodeDebugging() for JavaScriptAppResource sets ScriptPath to an empty string. The extension’s node debugger implementation currently requires script_path to compute program/cwd/displayName, so this will fail for AddJavaScriptApp/AddViteApp scenarios. Consider setting script_path to something non-empty (e.g., the app working directory or a specific entry file) for package-manager based apps.
Comment on lines
944
to
965
| [Experimental("ASPIREEXTENSION001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| internal static IResourceBuilder<T> WithVSCodeDebugging<T>(this IResourceBuilder<T> builder, string scriptPath) | ||
| where T : NodeAppResource | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); | ||
| ArgumentException.ThrowIfNullOrEmpty(scriptPath); | ||
|
|
||
| return builder.WithDebugSupport( | ||
| options => | ||
| { | ||
| var modeText = options.Mode == "Debug" ? "Debug" : "Run"; | ||
| var workspaceRoot = builder.ApplicationBuilder.Configuration[KnownConfigNames.ExtensionWorkspaceRoot]; | ||
| var displayPath = workspaceRoot is not null | ||
| ? Path.GetRelativePath(workspaceRoot, builder.Resource.WorkingDirectory) | ||
| : builder.Resource.WorkingDirectory; | ||
|
|
||
| // Check if a run script annotation is present - if so, use package manager instead of direct node | ||
| var hasRunScript = builder.Resource.TryGetLastAnnotation<JavaScriptRunScriptAnnotation>(out var runScriptAnnotation); | ||
| var hasPackageManager = builder.Resource.TryGetLastAnnotation<JavaScriptPackageManagerAnnotation>(out var pmAnnotation); | ||
| // Check if a run script annotation is present - if so, use package manager instead of direct node | ||
| var hasRunScript = builder.Resource.TryGetLastAnnotation<JavaScriptRunScriptAnnotation>(out _); | ||
| var hasPackageManager = builder.Resource.TryGetLastAnnotation<JavaScriptPackageManagerAnnotation>(out var pmAnnotation); | ||
|
|
||
| VSCodeNodeDebuggerProperties debuggerProperties; | ||
| string runtimeExecutable; | ||
|
|
||
| if (hasRunScript && hasPackageManager) | ||
| { | ||
| runtimeExecutable = pmAnnotation!.ExecutableName; | ||
|
|
||
| debuggerProperties = new VSCodeNodeDebuggerProperties | ||
| { | ||
| Name = $"{modeText} {runtimeExecutable}: {displayPath}", | ||
| WorkingDirectory = builder.Resource.WorkingDirectory, | ||
| RuntimeExecutable = runtimeExecutable, | ||
| RuntimeArgs = [], | ||
| ResolveSourceMapLocations = [$"{builder.Resource.WorkingDirectory}/**", "!**/node_modules/**"] | ||
| }; | ||
| } | ||
| else | ||
| { | ||
| // Direct node execution mode | ||
| runtimeExecutable = "node"; | ||
| debuggerProperties = new VSCodeNodeDebuggerProperties | ||
| { | ||
| Name = $"{modeText} Node.js: {displayPath}", | ||
| WorkingDirectory = builder.Resource.WorkingDirectory, | ||
| Program = Path.Combine(builder.Resource.WorkingDirectory, scriptPath), | ||
| RuntimeExecutable = runtimeExecutable, | ||
| ResolveSourceMapLocations = [$"{builder.Resource.WorkingDirectory}/**", "!**/node_modules/**"] | ||
| }; | ||
| } | ||
|
|
||
| if (builder.Resource.TryGetAnnotationsOfType<IDebuggerPropertiesAnnotation>(out var annotations)) | ||
| { | ||
| foreach (var annotation in annotations) | ||
| { | ||
| // Filter by IDE type if specified, and by debugger properties type | ||
| if (annotation.IdeType is null || AspireIde.IsCurrentIde(annotation.IdeType)) | ||
| { | ||
| annotation.ConfigureDebuggerProperties(debuggerProperties); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return new NodeLaunchConfiguration | ||
| { | ||
| ScriptPath = scriptPath, | ||
| Mode = options.Mode, | ||
| RuntimeExecutable = runtimeExecutable, | ||
| DebuggerProperties = debuggerProperties | ||
| }; | ||
| }, | ||
| "node"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Configures debugging support for a JavaScript resource that uses a package manager script. | ||
| /// </summary> | ||
| /// <typeparam name="T">The type of the resource.</typeparam> | ||
| /// <param name="builder">The resource builder.</param> | ||
| /// <returns>A reference to the <see cref="IResourceBuilder{T}"/> for method chaining.</returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// This method enables debugging for JavaScript applications that run via package manager scripts | ||
| /// (e.g., <c>npm run dev</c>) when running in the VS Code extension. | ||
| /// The debug configuration uses the package manager as the runtime executable. | ||
| /// </para> | ||
| /// <para> | ||
| /// This method is called automatically by <see cref="AddJavaScriptApp"/> and <see cref="AddViteApp"/>. | ||
| /// </para> | ||
| /// </remarks> | ||
| [Experimental("ASPIREEXTENSION001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| internal static IResourceBuilder<T> WithVSCodeDebugging<T>(this IResourceBuilder<T> builder) | ||
| where T : JavaScriptAppResource | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); | ||
| var runtimeExecutable = hasRunScript && hasPackageManager ? pmAnnotation!.ExecutableName : "node"; | ||
|
|
||
| return builder.WithDebugSupport( | ||
| options => | ||
| mode => new NodeLaunchConfiguration | ||
| { | ||
| var modeText = options.Mode == "Debug" ? "Debug" : "Run"; | ||
| var workspaceRoot = builder.ApplicationBuilder.Configuration[KnownConfigNames.ExtensionWorkspaceRoot]; | ||
| var displayPath = workspaceRoot is not null | ||
| ? Path.GetRelativePath(workspaceRoot, builder.Resource.WorkingDirectory) | ||
| : builder.Resource.WorkingDirectory; | ||
|
|
||
| // Get package manager info for display name and runtime executable | ||
| var packageManager = "npm"; | ||
|
|
||
| if (builder.Resource.TryGetLastAnnotation<JavaScriptPackageManagerAnnotation>(out var pmAnnotation)) | ||
| { | ||
| packageManager = pmAnnotation.ExecutableName; | ||
| } | ||
|
|
||
| var debuggerProperties = new VSCodeNodeDebuggerProperties | ||
| { | ||
| Name = $"{modeText} {packageManager}: {displayPath}", | ||
| WorkingDirectory = builder.Resource.WorkingDirectory, | ||
| RuntimeExecutable = packageManager, | ||
| RuntimeArgs = [], | ||
| ResolveSourceMapLocations = [$"{builder.Resource.WorkingDirectory}/**", "!**/node_modules/**"] | ||
| }; | ||
|
|
||
| if (builder.Resource.TryGetAnnotationsOfType<IDebuggerPropertiesAnnotation>(out var annotations)) | ||
| { | ||
| foreach (var annotation in annotations) | ||
| { | ||
| // Filter by IDE type if specified, and by debugger properties type | ||
| if (annotation.IdeType is null || AspireIde.IsCurrentIde(annotation.IdeType)) | ||
| { | ||
| annotation.ConfigureDebuggerProperties(debuggerProperties); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return new NodeLaunchConfiguration | ||
| { | ||
| ScriptPath = string.Empty, | ||
| Mode = options.Mode, | ||
| RuntimeExecutable = packageManager, | ||
| DebuggerProperties = debuggerProperties | ||
| }; | ||
| ScriptPath = scriptPath, | ||
| Mode = mode, | ||
| RuntimeExecutable = runtimeExecutable | ||
| }, | ||
| "node"); | ||
| } |
There was a problem hiding this comment.
WithVSCodeDebugging(builder, scriptPath) forwards the provided scriptPath directly into the launch configuration. scriptPath is typically relative (e.g., "app.js"), but the extension uses it as the program and derives cwd from path.dirname(script_path), which will resolve incorrectly. Consider emitting an absolute path (e.g., Path.GetFullPath(scriptPath, builder.Resource.WorkingDirectory)) and using the argsCallback hook to remove the script path from the resource args in IDE mode (since VS Code’s node debug config uses program separately).
Comment on lines
+36
to
+41
| // For package manager script execution (e.g., npm run dev), configure runtimeArgs | ||
| if (launchConfig.runtime_executable && launchConfig.runtime_executable !== 'node') { | ||
| debugConfiguration.runtimeArgs = ['run', ...(args ?? [])]; | ||
| delete debugConfiguration.args; | ||
| delete debugConfiguration.program; | ||
| } |
There was a problem hiding this comment.
For package-manager execution, runtimeArgs is built as ['run', ...(args ?? [])], but the apphost already includes the script command (often run) in payload.args (e.g., run dev). This can result in duplicated arguments like run run dev. Consider using args as-is, or only prepending 'run' when it’s not already the first arg.
Comment on lines
10
to
13
| <ItemGroup> | ||
| <ProjectReference Include="..\Aspire.Hosting\Aspire.Hosting.csproj" /> | ||
| <Compile Include="$(SharedDir)PathNormalizer.cs" Link="Utils\PathNormalizer.cs" /> | ||
| <Compile Include="$(RepoRoot)src\Aspire.Hosting\Dcp\Model\ExecutableLaunchConfiguration.cs" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
The <Compile Include="$(RepoRoot)src\Aspire.Hosting\Dcp\Model\ExecutableLaunchConfiguration.cs" /> path uses backslashes and isn’t linked into the project. For cross-platform builds it’s safer to use forward slashes and/or Link=... (or move this file under $(SharedDir)), otherwise path handling can be brittle on non-Windows agents.
Comment on lines
+10
to
+14
| <ItemGroup> | ||
| <Compile Include="$(SharedDir)PathNormalizer.cs" Link="Utils\PathNormalizer.cs" /> | ||
| <Compile Include="$(SharedDir)OverloadResolutionPriorityAttribute.cs" Link="Utils\OverloadResolutionPriorityAttribute.cs" /> | ||
| <Compile Include="$(RepoRoot)src\Aspire.Hosting\Dcp\Model\ExecutableLaunchConfiguration.cs" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
The <Compile Include="$(RepoRoot)src\Aspire.Hosting\Dcp\Model\ExecutableLaunchConfiguration.cs" /> path uses backslashes and isn’t linked into the project. For cross-platform builds it’s safer to use forward slashes and/or Link=... (or move this file under $(SharedDir)), otherwise path handling can be brittle on non-Windows agents.
Comment on lines
53
to
62
| try | ||
| { | ||
| var taskFunction = await _extensionTaskChannel.Reader.ReadAsync().ConfigureAwait(false); | ||
| await taskFunction.Invoke(); | ||
| } | ||
| catch (Exception ex) when (IsExtensionConnectionLostException(ex)) | ||
| { | ||
| // Connection was lost - only log to console, don't try to send over the closed connection | ||
| _consoleInteractionService.DisplaySubtleMessage(ex.Message); | ||
| } | ||
| catch (Exception ex) when (ex is not ExtensionOperationCanceledException) | ||
| { | ||
| // Try to display error to extension, but if that fails due to connection issues, just log to console | ||
| try | ||
| { | ||
| await Backchannel.DisplayErrorAsync(ex.Message.RemoveSpectreFormatting(), _cancellationToken); | ||
| } | ||
| catch (Exception innerEx) when (IsExtensionConnectionLostException(innerEx)) | ||
| { | ||
| // Swallow connection lost exceptions when trying to report the error | ||
| } | ||
| await Backchannel.DisplayErrorAsync(ex.Message.RemoveSpectreFormatting(), _cancellationToken); | ||
| _consoleInteractionService.DisplayError(ex.Message); | ||
| } |
There was a problem hiding this comment.
In the background task loop, the catch block calls await Backchannel.DisplayErrorAsync(...) without guarding against connection-lost errors. If the extension connection has already shut down, this can throw and terminate the loop task, potentially breaking subsequent UI notifications. Consider restoring the previous connection-lost handling (swallow/log to console) around DisplayErrorAsync.
Comment on lines
+3153
to
+3162
| /// <summary> | ||
| /// Adds support for debugging the resource in VS Code when running in an extension host. | ||
| /// </summary> | ||
| /// <param name="builder">The resource builder.</param> | ||
| /// <param name="launchConfigurationProducer">Launch configuration producer for the resource.</param> | ||
| /// <param name="launchConfigurationType">The type of the resource.</param> | ||
| /// <param name="argsCallback">Optional callback to add or modify command line arguments when running in an extension host. Useful if the entrypoint is usually provided as an argument to the resource executable.</param> | ||
| [Experimental("ASPIREEXTENSION001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| public static IResourceBuilder<T> WithDebugSupport<T, TLaunchConfiguration>(this IResourceBuilder<T> builder, Func<string, TLaunchConfiguration> launchConfigurationProducer, string launchConfigurationType, Action<CommandLineArgsCallbackContext>? argsCallback = null) | ||
| where T : IResource |
There was a problem hiding this comment.
The PR description states “no new public APIs”, but WithDebugSupport is now public on ResourceBuilderExtensions (it was previously internal). If the intent is to keep debugging support infrastructure internal, this should remain internal (or be explicitly called out/approved as a public API addition).
Comment on lines
+3153
to
+3163
| /// <summary> | ||
| /// Adds support for debugging the resource in VS Code when running in an extension host. | ||
| /// </summary> | ||
| /// <param name="builder">The resource builder.</param> | ||
| /// <param name="launchConfigurationProducer">Launch configuration producer for the resource.</param> | ||
| /// <param name="launchConfigurationType">The type of the resource.</param> | ||
| /// <param name="argsCallback">Optional callback to add or modify command line arguments when running in an extension host. Useful if the entrypoint is usually provided as an argument to the resource executable.</param> | ||
| [Experimental("ASPIREEXTENSION001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| public static IResourceBuilder<T> WithDebugSupport<T, TLaunchConfiguration>(this IResourceBuilder<T> builder, Func<string, TLaunchConfiguration> launchConfigurationProducer, string launchConfigurationType, Action<CommandLineArgsCallbackContext>? argsCallback = null) | ||
| where T : IResource | ||
| { |
There was a problem hiding this comment.
WithDebugSupport is a public builder extension method but it doesn’t have [AspireExport] or [AspireExportIgnore]. This will trigger analyzer ASPIRE014 (warnings-as-errors) during build; either mark it with [AspireExportIgnore] (if it’s meant to stay internal to .NET) or add [AspireExport] with an explicit export ID (and ensure the API is intended to be public).
Comment on lines
+1326
to
1331
| if (executable.SupportsDebugging(_configuration, out var supportsDebuggingAnnotation)) | ||
| { | ||
| // Just mark as IDE execution here - the actual callback will be invoked | ||
| // in CreateExecutableAsync after endpoints are allocated | ||
| exe.Spec.ExecutionType = ExecutionType.IDE; | ||
| exe.Spec.FallbackExecutionTypes = [ExecutionType.Process]; | ||
| isProcessExecution = false; | ||
| exe.Spec.FallbackExecutionTypes = [ ExecutionType.Process ]; | ||
| supportsDebuggingAnnotation.LaunchConfigurationAnnotator(exe, _configuration[KnownConfigNames.DebugSessionRunMode] ?? ExecutableLaunchMode.NoDebug); | ||
| } |
There was a problem hiding this comment.
supportsDebuggingAnnotation.LaunchConfigurationAnnotator(...) is invoked during PreparePlainExecutables() without any exception handling. If a launch configuration producer throws, it can fail the entire run (and you removed the later try/catch fallback). Wrap this call in try/catch and fall back to ExecutionType.Process (and ideally log) to preserve the previous resilience behavior.
Comment on lines
+1374
to
+1389
| var projectLaunchConfiguration = new ProjectLaunchConfiguration(); | ||
| projectLaunchConfiguration.ProjectPath = projectMetadata.ProjectPath; | ||
|
|
||
| var projectArgs = new List<string>(); | ||
|
|
||
| var isProcessExecution = true; | ||
| if (project.SupportsDebugging(_configuration, out _)) | ||
| { | ||
| // Just mark as IDE execution here - the actual callback will be invoked | ||
| // in CreateExecutableAsync after endpoints are allocated | ||
| exe.Spec.ExecutionType = ExecutionType.IDE; | ||
| exe.Spec.FallbackExecutionTypes = [ExecutionType.Process]; | ||
| isProcessExecution = false; | ||
| } | ||
| exe.Spec.FallbackExecutionTypes = [ ExecutionType.Process ]; | ||
|
|
||
| if (isProcessExecution) | ||
| projectLaunchConfiguration.DisableLaunchProfile = project.TryGetLastAnnotation<ExcludeLaunchProfileAnnotation>(out _); | ||
| // Use the effective launch profile which has fallback logic | ||
| if (!projectLaunchConfiguration.DisableLaunchProfile && project.GetEffectiveLaunchProfile() is NamedLaunchProfile namedLaunchProfile) | ||
| { | ||
| projectLaunchConfiguration.LaunchProfile = namedLaunchProfile.Name; | ||
| } |
There was a problem hiding this comment.
ProjectLaunchConfiguration.Mode is never set based on ASPIRE_EXTENSION_DEBUG_RUN_MODE (or KnownConfigNames.DebugSessionRunMode). The VS Code extension uses launchConfig.mode === "Debug" to decide whether to start a debug session, so projects will effectively always be treated as NoDebug here. Consider setting projectLaunchConfiguration.Mode from configuration (and/or using the SupportsDebuggingAnnotation launch-config producer for project resources, so the apphost-provided producer isn’t ignored).
- BrowserLaunchConfiguration: launch config extending ExecutableLaunchConfiguration('browser')
with url, webRoot, and browser properties
- BrowserDebuggerResource: internal child resource for browser debugging
- WithBrowserDebugger: creates child resource with WithDebugSupport for browser capability
- ValidateBrowserCapability: validates extension supports browser debugging
- browser.ts: extension debugger using VS Code built-in pwa-msedge/chrome adapter
- Added 'browser' capability to extension capabilities
- Fixed duplicate 'run' in node.ts: args from DCP already contain the full command,
so runtimeArgs should use args directly instead of prepending 'run'
- Fixed getProjectFile to handle empty script_path for package manager mode
adamint
requested review from
eerhardt,
jfversluis,
radical and
sebastienros
as code owners
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.
Description
Minimal reimplementation of JavaScript/Node.js debugging support, including browser debugging. This PR is based on the revert of PR #14686 (#15064) and adds back JS debugging with minimal changes and no new public APIs.
Node.js Debugging
NodeLaunchConfiguration: Internal launch config class extendingExecutableLaunchConfiguration("node")withScriptPathandRuntimeExecutablepropertiesWithVSCodeDebugginginternal methods: Two overloads wired intoAddNodeAppandCreateDefaultJavaScriptAppBuilder, calling the existingWithDebugSupportinfrastructurenode.tsimplementingResourceDebuggerExtensionusing VS Code's built-in js-debug adapterBrowser Debugging
BrowserLaunchConfiguration: Internal launch config extendingExecutableLaunchConfiguration("browser")withUrl,WebRoot, andBrowserpropertiesBrowserDebuggerResource: Internal child resource extendingExecutableResource, created as a child of the JS appWithBrowserDebuggerinternal method: Creates a child browser debugger resource withWithDebugSupport, waits for parent, resolves endpoint URLbrowser.tsimplementingResourceDebuggerExtensionusing VS Code's built-inpwa-msedge/pwa-chromeadapter"browser"capability before creating the browser resourceBug Fixes
node.tswas prepending"run"toruntimeArgswhen args from DCP already contained the full command (e.g.,["run", "dev", "--port", "5173"]), resulting in["run", "run", "dev", ...]getProjectFile: Now handles emptyscript_pathfor package manager mode instead of throwingDesign Decisions
WithDebugging,WithBrowserDebuggerpublic methods, etc.)WithDebugSupport+ExecutableLaunchConfigurationflowdebugger_propertiesextensibility systemCommandLineArgsCallbackAnnotationfromWithDebugSupportDepends on #15064 (revert of #14686)
Checklist