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

Debugger throws when debugging code which references a source map but the map file is missing. #13336

Closed
dbaeumer opened this issue Oct 7, 2016 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release debug Debug viewlet, configurations, breakpoints, adapter issues important Issue identified as high-priority verified Verification succeeded windows VS Code on Windows issues

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Oct 7, 2016

  • VSCode Version: 1.6.0-insider
  • OS Version:

Steps to Reproduce:

  1. install vscode-jsonrpc@^3.0.0-alpha.1
  2. install @type/node
  3. create tsconfig.json
{
    "compilerOptions": {
        "module": "commonjs",
        "sourceMap": true
    }
}
  1. create app.ts
import * as rpc from 'vscode-jsonrpc';

let logger: rpc.Logger  = {
    error: (message) => {},
    info: (message) => {},
    warn: (message) => {},
    log: (message) => {}
}

let connection = rpc.createMessageConnection(process.stdin, process.stdout, logger);
  1. compile using tsc. ignore the compile errors. Not relevant for the problem
  2. create launch.json and ENABLE source maps
  3. set breakpoint in app.ts
  4. debug
  5. try to step into createMessageConnection

You get (see the exception on the call stack). I am pretty sure that before the debugger simply stepped into the JS code.

capture

@dbaeumer dbaeumer added debug Debug viewlet, configurations, breakpoints, adapter issues important Issue identified as high-priority labels Oct 7, 2016
@isidorn
Copy link
Contributor

isidorn commented Oct 7, 2016

Works for me on OSX - trying on windows...

stepinjs

@bpasero
Copy link
Member

bpasero commented Oct 7, 2016

@isidorn keep @dbaeumer special subst configuration on Windows in mind.

@isidorn
Copy link
Contributor

isidorn commented Oct 7, 2016

@dbaeumer works for me on windows also. What version of node do you have? Do you also see this with our Visual studio stable?
@bpasero might be dirk's special setup - in that case will remove the important tag and push to october

stepinjs

@isidorn isidorn added the info-needed Issue requires more information from poster label Oct 7, 2016
@dbaeumer
Copy link
Member Author

dbaeumer commented Oct 7, 2016

I have node 6.5.0.

@bpasero I am pretty sure that this has nothing to do with the subst setup. It looks for a file on disk that doesn't exist and throws. I tested it with opening the same workspace from C:\ and it throws in the same way.

@dbaeumer
Copy link
Member Author

dbaeumer commented Oct 7, 2016

@isidorn you need to step into 'createMessageConnection'. In both screen casts you are stepping into process.stdin / stdout getters. You need to step out and step in again. May be more times. Sorry for not making this more explicit.

@isidorn
Copy link
Contributor

isidorn commented Oct 7, 2016

@dbaeumer thanks for the clarification. I can repro on Win, not on Mac.
Does not repro with stable vscode - it is a regression in node debug, investigating...

fyi @roblourens

@isidorn isidorn added bug Issue identified by VS Code Team member as probable bug windows VS Code on Windows issues and removed info-needed Issue requires more information from poster labels Oct 7, 2016
@isidorn isidorn added the candidate Issue identified as probable candidate for fixing in the next release label Oct 7, 2016
@isidorn isidorn added this to the September Recovery 2016 milestone Oct 7, 2016
@isidorn
Copy link
Contributor

isidorn commented Oct 7, 2016

Ok pushed a fix which handles the error better and fixes the issue on windows.
Please code review @roblourens @weinand since I do not have deep understanding of node-debug code

isidorn added a commit that referenced this issue Oct 7, 2016
isidorn added a commit that referenced this issue Oct 7, 2016
@roblourens
Copy link
Member

roblourens commented Oct 7, 2016

This will also show up if that function fails for any other reason, like if the sourcemap path is a url that doesn't exist, or invalid inlined base64. Maybe that's less likely but I can try to move the error handling upstream.

image

roblourens added a commit to microsoft/vscode-node-debug that referenced this issue Oct 7, 2016
@roblourens
Copy link
Member

I pushed a fix for this that catches anything outside of _loadSourceMap... not sure if I should put it in 1.6 or not?

@kieferrm kieferrm added the verified Verification succeeded label Oct 7, 2016
@isidorn
Copy link
Contributor

isidorn commented Oct 8, 2016

@roblourens for now I would go with the original fix since it is smaller and already verified. Unles you see some direct benefits of going with you proposed solution for the current release?
Thanks for looking into it!

@roblourens
Copy link
Member

Nah, let's go with the original fix for 1.6 and keep the other for later.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release debug Debug viewlet, configurations, breakpoints, adapter issues important Issue identified as high-priority verified Verification succeeded windows VS Code on Windows issues
Projects
None yet
Development

No branches or pull requests

7 participants