Implement "Debug Locally" code lens for .NET Core. #552

mpiroc · 2019-05-10T00:41:31Z

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Infrastructure (change which improves the lifecycle management (CI/CD, build, package, deploy, lint, etc) of the application, but does not change functionality.)
Technical debt (change which improves the maintainability of the codebase, but does not change functionality)
Testing (change which modifies or adds test coverage, but does not change functionality)
Documentation (change which modifies or adds documentation, but does not change functionality)

Description

See title.

Motivation and Context

Debugging lambda functions locally is part of the core feature set of the toolkit. We do not consider a runtime to be supported if it cannot be debugged locally.

Related Issue(s)

#514

Testing

Manually tested locally on Windows only.
Ran all new and existing tests.

Checklist

I have read the README document
I have read the CONTRIBUTING document
My code follows the code style of this project
I have added tests to cover my changes
All new and existing tests passed
A short description of the change has been added to the CHANGELOG

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

mpiroc · 2019-05-10T00:41:51Z

TODO

Add changelog entry.
Unit tests for makeCoreCLRDebugConfiguration
Test on MacOS and Ubuntu.

codecov-io · 2019-05-10T15:40:52Z

Codecov Report

Merging #552 into feature/dotnet will decrease coverage by 0.02%.
The diff coverage is 40.9%.

@@                Coverage Diff                @@
##           feature/dotnet    #552      +/-   ##
=================================================
- Coverage           52.92%   52.9%   -0.03%     
=================================================
  Files                 108     110       +2     
  Lines                4287    4355      +68     
  Branches              625     632       +7     
=================================================
+ Hits                 2269    2304      +35     
- Misses               2018    2051      +33

Impacted Files	Coverage Δ
src/shared/codelens/localLambdaRunner.ts	`29.68% <ø> (ø)`	⬆️
src/shared/codelens/pythonCodeLensProvider.ts	`0% <0%> (ø)`	⬆️
src/shared/codelens/codeLensUtils.ts	`28% <100%> (+1.46%)`	⬆️
src/shared/codelens/csharpCodeLensProvider.ts	`45.6% <17.77%> (-5.92%)`	⬇️
src/shared/clients/dockerClient.ts	`60.6% <60.6%> (ø)`
src/lambda/local/debugConfiguration.ts	`87.5% <87.5%> (ø)`
src/shared/logger.ts	`89.65% <0%> (+1.14%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6855f0...a7fb3f6. Read the comment docs.

bryceitoc9 · 2019-05-10T16:24:53Z

src/lambda/local/debugConfiguration.ts

+            debuggerPath,
+            pipeCwd: codeUri
+        },
+        windows: {


Is it necessary to have two separate pipeTransports? You've already determined the OS above, should that define what pipeTransport is added to this JSON?

Is isn't necessary since this configuration isn't saved anywhere (i.e. will only run on the current platform). But I wanted to minimize differences between the configuration that would appear in launch.json (see the SAM CLI docs) and this configuration.

bryceitoc9 · 2019-05-10T16:28:16Z

src/shared/codelens/csharpCodeLensProvider.ts

-                handlerFileRelativePath
-            })
+        const baseBuildDir = await makeBuildDir()
+        const codeDir = path.dirname(lambdaLocalInvokeParams.document.uri.fsPath)


Can you confirm that this works with a nested case? Test against directory structures similar to the following:

Fixed ✔️

bryceitoc9 · 2019-05-10T19:34:13Z

src/shared/codelens/csharpCodeLensProvider.ts

@@ -380,3 +400,60 @@ export function isPublicMethodSymbol(
 export function generateDotNetLambdaHandler(components: DotNetLambdaHandlerComponents): string {
    return `${components.assembly}::${components.namespace}.${components.class}::${components.method}`
 }
+
+export async function installDebugger({


Does the debugger already exist in the Docker image? If so, can we hijack that instead of downloading ourselves?

Nope, it doesn't exist in the image :(

src/shared/codelens/csharpCodeLensProvider.ts

src/lambda/local/debugConfiguration.ts

awschristou · 2019-05-13T15:32:35Z

src/lambda/local/debugConfiguration.ts

+        name: '.NET Core Docker Attach',
+        type: 'coreclr',
+        request: 'attach',
+        processId: '1',


Is processId necessary? This seems like a magic value

This is taken directly from the SAM CLI docs. Checking with SAM CLI folks to determine if it is necessary...

src/shared/codelens/csharpCodeLensProvider.ts

src/lambda/local/debugConfiguration.ts

src/shared/codelens/csharpCodeLensProvider.ts

src/shared/clients/dockerClient.ts

…y. (#599) * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

Due to a series of misteps, we are squashing this entire feature branch into develop. 1. We allowed `feature/dotnet` to drift too far from `develop`, which required significant manual conflict resolution to resolve. 2. We should have squashed the conflict-resolution history *before* merging develop, but we did not. Therefore in order to squash the pre-merge commits, we would need to recreate the merge and re-resolve conflicts. This would reduce our confidence in the quality of the branch. In the future: 1. We will regularly merge into feature branches to prevent drift. 2. If we do encounter a situation that involves manually porting, we will squash the ports *before* merging develop. * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

Implement "Debug Locally" code lens for .NET Core.

11a58b6

mpiroc requested review from bryceitoc9 and awschristou May 10, 2019 00:41

mpiroc self-assigned this May 10, 2019

bryceitoc9 reviewed May 10, 2019

View reviewed changes

src/shared/codelens/csharpCodeLensProvider.ts Outdated Show resolved Hide resolved

bryceitoc9 mentioned this pull request May 10, 2019

Users can skip pulling container images #376

Closed

awschristou requested changes May 13, 2019

View reviewed changes

mpiroc added 3 commits May 20, 2019 14:46

Address feedback from PR.

a674976

Add tests for makeCoreCLRDebugConfiguration.

e2ae962

Add tests for DefaultDockerClient.

1ab0726

awschristou requested changes May 21, 2019

View reviewed changes

src/lambda/local/debugConfiguration.ts Outdated Show resolved Hide resolved

src/shared/codelens/csharpCodeLensProvider.ts Outdated Show resolved Hide resolved

src/shared/clients/dockerClient.ts Outdated Show resolved Hide resolved

mpiroc added 4 commits May 21, 2019 17:08

Fix order of docker arguments.

436ad33

Address feedback from PR.

63e3b98

Fix failing test.

d0c048a

Fix codelenses when source file is not a sibling of project file.

a7fb3f6

awschristou approved these changes May 22, 2019

View reviewed changes

bryceitoc9 approved these changes May 23, 2019

View reviewed changes

mpiroc merged commit 4e23366 into feature/dotnet May 28, 2019

mpiroc deleted the pirocchi/dotnet/debug-impl2 branch May 28, 2019 17:28

CALISOULB mentioned this pull request Apr 9, 2023

[Snyk] Security upgrade xml2js from 0.4.23 to 0.5.0 CALISOULB/aws-toolkit-vscode#77

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement "Debug Locally" code lens for .NET Core. #552

Implement "Debug Locally" code lens for .NET Core. #552

mpiroc commented May 10, 2019

mpiroc commented May 10, 2019 •

edited

Loading

codecov-io commented May 10, 2019 •

edited

Loading

bryceitoc9 May 10, 2019

mpiroc May 20, 2019

bryceitoc9 May 10, 2019

mpiroc May 22, 2019

bryceitoc9 May 10, 2019

mpiroc May 20, 2019

awschristou May 13, 2019

mpiroc May 20, 2019

Implement "Debug Locally" code lens for .NET Core. #552

Implement "Debug Locally" code lens for .NET Core. #552

Conversation

mpiroc commented May 10, 2019

Types of changes

Description

Motivation and Context

Related Issue(s)

Testing

Checklist

License

mpiroc commented May 10, 2019 • edited Loading

TODO

codecov-io commented May 10, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpiroc commented May 10, 2019 •

edited

Loading

codecov-io commented May 10, 2019 •

edited

Loading