Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Commit

Permalink
fix: breakpoints work on windows (#815)
Browse files Browse the repository at this point in the history
On Windows, on Node 11+, the URL provided to the inspector API must start with
file:/// instead of file:// and all backslashes must be converted to forward slashes.
Otherwise, on Node 10 on windows, the URL must not start with file:// and must
contain backslashes.

Fixes: #795
  • Loading branch information
DominicKramer committed Jan 23, 2020
1 parent d5a17b2 commit 8309839
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 17 deletions.
14 changes: 7 additions & 7 deletions .kokoro/test.bat

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 23 additions & 4 deletions src/agent/state/inspector-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ const GETTER_MESSAGE_INDEX = 2;
const ARG_LOCAL_LIMIT_MESSAGE_INDEX = 3;

const FILE_PROTOCOL = 'file://';
// on windows on Node 11+ the file protocol needs to have three slashes
const WINDOWS_FILE_PROTOCOL = 'file:///';

// Used to match paths like file:///C:/... on windows
// but do not match paths like file:///home/... on linux
const WINDOWS_URL_REGEX = RegExp(`^${WINDOWS_FILE_PROTOCOL}[a-zA-Z]+:`);

const STABLE_OBJECT_ID_PROPERTY = '[[StableObjectId]]';
const NO_STABLE_OBJECT_ID = -1;
Expand Down Expand Up @@ -316,7 +322,13 @@ class StateResolver {
const scriptUrl = this.scriptmapper[scriptId].url;
// In Node 11+, non-internal files are formatted as URLs, so get just the
// path.
return StateResolver.stripFileProtocol_(scriptUrl);
const strippedUrl = StateResolver.stripFileProtocol_(scriptUrl);
if (process.platform === 'win32') {
// on windows the script url provided to v8 uses forward slashes
// convert them back to backslashes
return strippedUrl.replace(/\//g, '\\');
}
return strippedUrl;
}

resolveRelativePath_(frame: inspector.Debugger.CallFrame): string {
Expand All @@ -325,9 +337,16 @@ class StateResolver {
}

static stripFileProtocol_(path: string) {
return path.toLowerCase().startsWith(FILE_PROTOCOL)
? path.substr(FILE_PROTOCOL.length)
: path;
const lowerPath = path.toLowerCase();
if (WINDOWS_URL_REGEX.test(lowerPath)) {
return path.substr(WINDOWS_FILE_PROTOCOL.length);
}

if (lowerPath.startsWith(FILE_PROTOCOL)) {
return path.substr(FILE_PROTOCOL.length);
}

return path;
}

stripCurrentWorkingDirectory_(path: string): string {
Expand Down
8 changes: 7 additions & 1 deletion src/agent/v8/inspector-debugapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,15 @@ export class InspectorDebugApi implements debugapi.DebugApi {
let v8BreakpointId; // v8/inspector breakpoint id
if (!this.locationMapper[locationStr]) {
// The first time when a breakpoint was set to this location.
const url = this.inspectorOptions.useWellFormattedUrl
const rawUrl = this.inspectorOptions.useWellFormattedUrl
? `file://${matchingScript}`
: matchingScript;
// on windows on Node 11+, the url must start with file:///
// (notice 3 slashes) and have all backslashes converted into forward slashes
const url =
process.platform === 'win32' && utils.satisfies(process.version, '>=11')
? rawUrl.replace(/^file:\/\//, 'file:///').replace(/\\/g, '/')
: rawUrl;
const res = this.v8Inspector.setBreakpointByUrl({
lineNumber: line - 1,
url,
Expand Down
6 changes: 1 addition & 5 deletions synth.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@
logging.basicConfig(level=logging.DEBUG)
common_templates = gcp.CommonTemplates()
templates = common_templates.node_library()
# stop excluding '.kokoro/test.bat' as soon as we figure out why Node 10 tests
# have issues on Windows (see #686).
s.copy(templates, excludes=[
'.kokoro/test.bat'
])
s.copy(templates)

subprocess.run(['npm', 'install'])
subprocess.run(['npm', 'run', 'fix'])

0 comments on commit 8309839

Please sign in to comment.