fix: breakpoints work on windows (#815)

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

Author: DominicKramer

.kokoro/test.bat

@@ -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;
@@ -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 {
@@ -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 {

src/agent/state/inspector-state.ts

@@ -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,

src/agent/v8/inspector-debugapi.ts

@@ -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'])