Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct launch.json for node js debugging through vs code #704

Merged
merged 3 commits into from
Oct 12, 2018

Conversation

sriram-mv
Copy link
Contributor

Issue #, if available:

#555

Description of changes:

stopOnEntry flag allows breaking directly into user code, instead of the runtime bootstrap file.

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

@heitorlessa
Copy link
Contributor

heitorlessa commented Oct 10, 2018

AWESOME @thesriram ;)

Would you be able to update the PR to point out that this will work if the source code is in the root (where template.yaml is). However, if they have functions inside folders (i.e. function1/app.js, function2/app.js) then they'll need to specify that in localroot too.

Here's an example to debug NodeJS when using the boilerplate provided by SAM init:

sam init
cd sam-app
# Brings Visual Studio code for this app
code . 

Creates a new debugger configuration using the following:

        {
            "name": "SAM Hello World function - Node8",
            "type": "node",
            "request": "attach",
            "address": "localhost",
            "port": 5858,
            "localRoot": "${workspaceRoot}/hello_world",
            "remoteRoot": "/var/task",
            "protocol": "inspector",
            "sourceMapPathOverrides": {
                "${workspaceRoot}/runtime": "${remoteRoot}"
            },
            "stopOnEntry": false
        }

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

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

At what point does the previous config we had not work? Can we clearly document which version(s) of VSCode the config you added supports? We should probably keep the one one around too and say use that config if your VSCode version is lower than X?

docs/usage.rst Outdated
"localRoot": "${workspaceRoot}",
"sourceMapPathOverrides": {
"${workspaceRoot}/runtime": "${remoteRoot}"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you format the JSON here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely.

docs/usage.rst Outdated
"${workspaceRoot}/runtime": "${remoteRoot}"
},
"remoteRoot": "/var/task",
"protocol": "inspector",
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous protocol was legacy. What does updating this to inspector indicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is for newer version of nodejs, that is indicated right after in the readme, to swtich to legacy if you are using a older version of nodejs.

docs/usage.rst Outdated
"request": "attach",
"address": "localhost",
"port": 5858,
"localRoot": "${workspaceRoot}",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be the ${workspaceRoot}/ right? This localRoot only works if you open the directory of the function code and not the whole project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes correct.

@sriram-mv
Copy link
Contributor Author

At what point does the previous config we had not work? Can we clearly document which version(s) of VSCode the config you added supports? We should probably keep the one one around too and say use that config if your VSCode version is lower than X?

It's a change in the way the first file the debugger tries to break into. we break into the code_uri directly with the stopOnEntry flag. Also this flag seems to exist from 2016, so it should definitely be backwards compatible. Yes this is VS code v1.27.2, I can add the version this was tested on.

- Remove source path mapping
- Add a note for the localRoot being based on the where the function
  code resides. This is referenced by CodeUri inside template.yaml
- Add a note on where VS code needs to be opened.
@jfuss jfuss merged commit 504ca32 into aws:develop Oct 12, 2018
@sriram-mv sriram-mv mentioned this pull request Oct 19, 2018
heitorlessa added a commit to heitorlessa/aws-sam-cli that referenced this pull request Oct 30, 2018
* develop:
  chore: Version bump aws-sam-translator to 1.8.0 (aws#732)
  docs: add instructions for using local version of SAM Transformer (aws#688)
  docs: Update Docker command to docker (aws#725)
  fix: Iterate over query param list
  chore: Enable Travis to run integ tests
  chore(0.6.1): SAM CLI Version bump (aws#717)
  fix(init/readme): Correct permissions for AWS CLI under requirements (aws#713)
  add pytest-mock for testing (aws#703)
  fix: allow for stdout and stderr streams to be unbufferred directly. (aws#708)
  docs: Add installation instructions for linux (Centos) (aws#670)
  fix: Updated isBase64Encoded value to bool (aws#699)
  fix: correct launch.json for nodejs debugging through VSCode (aws#704)
  docs(usage): Update how to debug Python functions using VS Code (aws#694)
  docs(Cloud9): Reset bash cache on Cloud9 (aws#693)
  docs: Updated virtualenv alias name for 3.7 in guide. (aws#706)
  chore: update aws-sam-translator to 1.7.0 (aws#682)
  feat: travis CI support for Python 3.7 (aws#679)
  docs: Update generate-sample-event-payloads link (aws#702)
  Fix: Raise error for invalid environment variables file (aws#675)
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.

None yet

3 participants