Bind unverified breakpoints when the sourcemap can only be found afte…

…r the script is loaded -

Fix #106

.vscode/launch.json

@@ -15,7 +15,7 @@
         // Replace the paths below with the paths to your own project.
         {
             "name": "launch as server",
-            "type": "node",
+            "type": "node2",
             "request": "launch",
 
             // These paths are only valid for my particular setup! You need to replace them with your own.
@@ -32,7 +32,7 @@
                 "program": "${workspaceRoot}/../vscode-node-debug2/out/src/nodeDebug.js"
             },
             "stopOnEntry": false,
-            "args": [ "--server=4712" ],
+            "args": [ "--server=4711" ],
             "sourceMaps": true,
             "outDir": "${workspaceRoot}/out"
         }

src/chrome/chromeDebugAdapter.ts

@@ -46,6 +46,11 @@ export interface ISourceContainer {
     mappedPath?: string;
 }
 
+interface IPendingBreakpoint {
+    args: ISetBreakpointsArgs;
+    ids: number[];
+}
+
 export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
     private static THREAD_ID = 1;
     private static PAGE_PAUSE_MESSAGE = 'Paused in Visual Studio Code';
@@ -67,6 +72,7 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
 
     private _scriptsById: Map<Chrome.Debugger.ScriptId, Chrome.Debugger.Script>;
     private _scriptsByUrl: Map<string, Chrome.Debugger.Script>;
+    private _pendingBreakpointsByUrl: Map<string, IPendingBreakpoint>;
 
     protected _chromeConnection: ChromeConnection;
 
@@ -79,6 +85,7 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
     protected _attachMode: boolean;
 
     private _currentStep = Promise.resolve<void>();
+    private _nextUnboundBreakpointId = 0;
 
     public constructor({chromeConnection, lineNumberTransformer, sourceMapTransformer, pathTransformer }: IChromeDebugSessionOpts) {
         super();
@@ -88,6 +95,7 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
         this._variableHandles = new Handles<IVariableContainer>();
         this._breakpointIdHandles = new utils.ReverseHandles<string>();
         this._sourceHandles = new Handles<ISourceContainer>();
+        this._pendingBreakpointsByUrl = new Map<string, IPendingBreakpoint>();
 
         this._overlayHelper = new utils.DebounceHelper(/*timeoutMs=*/200);
 
@@ -336,7 +344,24 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
         this._scriptsByUrl.set(script.url, script);
 
         const mappedUrl = this._pathTransformer.scriptParsed(script.url);
-        this._sourceMapTransformer.scriptParsed(mappedUrl, script.sourceMapURL);
+        this._sourceMapTransformer.scriptParsed(mappedUrl, script.sourceMapURL).then(sources => {
+            if (sources) {
+                sources.forEach(source => {
+                    if (this._pendingBreakpointsByUrl.has(source)) {
+                        this.resolvePendingBreakpoints(this._pendingBreakpointsByUrl.get(source));
+                    }
+                });
+            }
+        });
+    }
+
+    private resolvePendingBreakpoints(pendingBP: IPendingBreakpoint): void {
+        this.setBreakpoints(pendingBP.args, 0).then(response => {
+            response.breakpoints.forEach((bp, i) => {
+                bp.id = pendingBP.ids[i];
+                this.sendEvent(new BreakpointEvent('new', bp));
+            });
+        });
     }
 
     protected onBreakpointResolved(params: Chrome.Debugger.BreakpointResolvedParams): void {
@@ -376,44 +401,62 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
     }
 
     public setBreakpoints(args: ISetBreakpointsArgs, requestSeq: number): Promise<ISetBreakpointsResponseBody> {
-        this._lineNumberTransformer.setBreakpoints(args);
-        let canSetBp = this._sourceMapTransformer.setBreakpoints(args, requestSeq);
-        if (!canSetBp) return Promise.resolve(this.unverifiedBpResponse(args, utils.localize('sourcemapping.fail.message', "Breakpoint ignored because generated code not found (source map problem?).")));
+        return this.validateBreakpointsPath(args)
+            .then(() => {
+                this._lineNumberTransformer.setBreakpoints(args);
+                this._sourceMapTransformer.setBreakpoints(args, requestSeq);
+                this._pathTransformer.setBreakpoints(args);
+
+                let targetScriptUrl: string;
+                if (args.source.path) {
+                    targetScriptUrl = args.source.path;
+                } else if (args.source.sourceReference) {
+                    const handle = this._sourceHandles.get(args.source.sourceReference);
+                    const targetScript = this._scriptsById.get(handle.scriptId);
+                    if (targetScript) {
+                        targetScriptUrl = targetScript.url;
+                    }
+                }
 
-        canSetBp = this._pathTransformer.setBreakpoints(args);
-        if (!canSetBp) return Promise.resolve(this.unverifiedBpResponse(args));
+                if (targetScriptUrl) {
+                    // DebugProtocol sends all current breakpoints for the script. Clear all scripts for the breakpoint then add all of them
+                    const setBreakpointsPFailOnError = this._setBreakpointsRequestQ
+                        .then(() => this.clearAllBreakpoints(targetScriptUrl))
+                        .then(() => this.addBreakpoints(targetScriptUrl, args.breakpoints))
+                        .then(responses => ({ breakpoints: this.chromeBreakpointResponsesToODPBreakpoints(targetScriptUrl, responses, args.breakpoints) }));
+
+                    const setBreakpointsPTimeout = utils.promiseTimeout(setBreakpointsPFailOnError, /*timeoutMs*/2000, 'Set breakpoints request timed out');
+
+                    // Do just one setBreakpointsRequest at a time to avoid interleaving breakpoint removed/breakpoint added requests to Chrome.
+                    // Swallow errors in the promise queue chain so it doesn't get blocked, but return the failing promise for error handling.
+                    this._setBreakpointsRequestQ = setBreakpointsPTimeout.catch(() => undefined);
+                    return setBreakpointsPTimeout.then(body => {
+                        this._sourceMapTransformer.setBreakpointsResponse(body, requestSeq);
+                        this._lineNumberTransformer.setBreakpointsResponse(body);
+                        return body;
+                    });
+                } else {
+                    return Promise.resolve(this.unverifiedBpResponse(args, utils.localize('bp.fail.noscript', `Can't find script for breakpoint request`)));
+                }
+            },
+            e => this.unverifiedBpResponse(args, e.message));
+    }
 
-        let targetScriptUrl: string;
-        if (args.source.path) {
-            targetScriptUrl = args.source.path;
-        } else if (args.source.sourceReference) {
-            const handle = this._sourceHandles.get(args.source.sourceReference);
-            const targetScript = this._scriptsById.get(handle.scriptId);
-            if (targetScript) {
-                targetScriptUrl = targetScript.url;
+    private validateBreakpointsPath(args: ISetBreakpointsArgs): Promise<void> {
+        if (!args.source.path) return Promise.resolve<void>();
+
+        return this._sourceMapTransformer.getGeneratedPathFromAuthoredPath(args.source.path).then(mappedPath => {
+            if (!mappedPath) {
+                return utils.errP(utils.localize('sourcemapping.fail.message', "Breakpoint ignored because generated code not found (source map problem?)."));
             }
-        }
 
-        if (targetScriptUrl) {
-            // DebugProtocol sends all current breakpoints for the script. Clear all scripts for the breakpoint then add all of them
-            const setBreakpointsPFailOnError = this._setBreakpointsRequestQ
-                .then(() => this.clearAllBreakpoints(targetScriptUrl))
-                .then(() => this.addBreakpoints(targetScriptUrl, args.breakpoints))
-                .then(responses => ({ breakpoints: this.chromeBreakpointResponsesToODPBreakpoints(targetScriptUrl, responses, args.breakpoints) }));
-
-            const setBreakpointsPTimeout = utils.promiseTimeout(setBreakpointsPFailOnError, /*timeoutMs*/2000, 'Set breakpoints request timed out');
-
-            // Do just one setBreakpointsRequest at a time to avoid interleaving breakpoint removed/breakpoint added requests to Chrome.
-            // Swallow errors in the promise queue chain so it doesn't get blocked, but return the failing promise for error handling.
-            this._setBreakpointsRequestQ = setBreakpointsPTimeout.catch(() => undefined);
-            return setBreakpointsPTimeout.then(body => {
-                this._sourceMapTransformer.setBreakpointsResponse(body, requestSeq);
-                this._lineNumberTransformer.setBreakpointsResponse(body);
-                return body;
-            });
-        } else {
-            return Promise.resolve(this.unverifiedBpResponse(args, utils.localize('bp.fail.noscript', `Can't find script for breakpoint request`)));
-        }
+            const targetPath = this._pathTransformer.getTargetPathFromClientPath(mappedPath);
+            if (!targetPath) {
+                return Promise.reject(undefined);
+            }
+
+            return undefined;
+        });
     }
 
     private unverifiedBpResponse(args: ISetBreakpointsArgs, message?: string): ISetBreakpointsResponseBody {
@@ -422,13 +465,17 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {
                 verified: false,
                 line: bp.line,
                 column: bp.column,
-                message
+                message,
+                id: this._breakpointIdHandles.create(this._nextUnboundBreakpointId++ + '')
             };
         });
 
-        const response = { breakpoints };
-        this._lineNumberTransformer.setBreakpointsResponse(response);
-        return response;
+        if (args.source.path) {
+            const ids = breakpoints.map(bp => bp.id);
+            this._pendingBreakpointsByUrl.set(args.source.path, { args, ids });
+        }
+
+        return { breakpoints };
     }
 
     public setFunctionBreakpoints(): Promise<any> {

src/chrome/chromeDebugSession.ts

@@ -65,7 +65,7 @@ export class ChromeDebugSession extends DebugSession {
 
         process.addListener('unhandledRejection', err => {
             // err is a DebugProtocol.Message or Error
-            const errMsg = err.format ? JSON.stringify(err) : err.toString();
+            const errMsg = err.stack ? err.stack : JSON.stringify(err);
             logger.error(`******** Error in DebugAdapter - Unhandled promise rejection: ${errMsg}`);
         });
     }

src/transformers/basePathTransformer.ts

@@ -35,4 +35,12 @@ export class BasePathTransformer {
 
     public stackTraceResponse(response: IStackTraceResponseBody): void {
     }
+
+    public getTargetPathFromClientPath(clientPath: string): string {
+        return clientPath;
+    }
+
+    public getClientPathFromTargetPath(targetPath: string): string {
+        return targetPath;
+    }
 }

src/transformers/baseSourceMapTransformer.ts

@@ -198,19 +198,23 @@ export class BaseSourceMapTransformer {
         }
     }
 
-    public scriptParsed(pathToGenerated: string, sourceMapURL: string): void {
+    public scriptParsed(pathToGenerated: string, sourceMapURL: string): Promise<string[]> {
         if (this._sourceMaps) {
             this._allRuntimeScriptPaths.add(pathToGenerated);
 
-            if (!sourceMapURL) return;
+            if (!sourceMapURL) return Promise.resolve();
 
             // Load the sourcemap for this new script and log its sources
-            this._sourceMaps.processNewSourceMap(pathToGenerated, sourceMapURL).then(() => {
+            return this._sourceMaps.processNewSourceMap(pathToGenerated, sourceMapURL).then(() => {
                 const sources = this._sourceMaps.allMappedSources(pathToGenerated);
                 if (sources) {
                     logger.log(`SourceMaps.scriptParsed: ${pathToGenerated} was just loaded and has mapped sources: ${JSON.stringify(sources) }`);
                 }
+
+                return sources;
             });
+        } else {
+            return Promise.resolve();
         }
     }
 
@@ -234,6 +238,11 @@ export class BaseSourceMapTransformer {
     }
 
     public getGeneratedPathFromAuthoredPath(authoredPath: string): Promise<string> {
-        return this._preLoad.then(() => this._sourceMaps.getGeneratedPathFromAuthoredPath(authoredPath));
+        if (!this._sourceMaps) return Promise.resolve(authoredPath);
+        return this._preLoad.then(() => {
+            // Find the generated path, or check whether this script is actually a runtime path - if so, return that
+            return this._sourceMaps.getGeneratedPathFromAuthoredPath(authoredPath) ||
+                (this._allRuntimeScriptPaths.has(authoredPath) ? authoredPath : null);
+        });
     }
 }

src/transformers/remotePathTransformer.ts

@@ -48,22 +48,22 @@ export class RemotePathTransformer extends BasePathTransformer {
 
     public setBreakpoints(args: ISetBreakpointsArgs): boolean {
         if (args.source.path) {
-            args.source.path = this.localToRemote(args.source.path);
+            args.source.path = this.getTargetPathFromClientPath(args.source.path);
         }
 
         return super.setBreakpoints(args);
     }
 
     public scriptParsed(scriptPath: string): string {
-        scriptPath = this.remoteToLocal(scriptPath);
+        scriptPath = this.getClientPathFromTargetPath(scriptPath);
         return super.scriptParsed(scriptPath);
     }
 
     public stackTraceResponse(response: IStackTraceResponseBody): void {
         response.stackFrames.forEach(frame => {
             const remotePath = frame.source.path;
             if (remotePath) {
-                const localPath = this.remoteToLocal(remotePath);
+                const localPath = this.getClientPathFromTargetPath(remotePath);
                 if (utils.existsSync(localPath)) {
                     frame.source.path = localPath;
                     frame.source.sourceReference = undefined;
@@ -77,7 +77,7 @@ export class RemotePathTransformer extends BasePathTransformer {
         return !!this._localRoot && !!this._remoteRoot && (path.posix.isAbsolute(remotePath) || path.win32.isAbsolute(remotePath));
     }
 
-    private remoteToLocal(remotePath: string): string {
+    public getClientPathFromTargetPath(remotePath: string): string {
         if (!this.shouldMapPaths(remotePath)) return remotePath;
 
         const relPath = relative(this._remoteRoot, remotePath);
@@ -88,7 +88,7 @@ export class RemotePathTransformer extends BasePathTransformer {
         return localPath;
     }
 
-    private localToRemote(localPath: string): string {
+    public getTargetPathFromClientPath(localPath: string): string {
         if (!this.shouldMapPaths(localPath)) return localPath;
 
         const relPath = relative(this._localRoot, localPath);

src/transformers/urlPathTransformer.ts

@@ -40,8 +40,9 @@ export class UrlPathTransformer extends BasePathTransformer {
         }
 
         const path = utils.canonicalizeUrl(args.source.path);
-        if (this._clientPathToTargetUrl.has(path)) {
-            args.source.path = this._clientPathToTargetUrl.get(path);
+        const url = this.getTargetPathFromClientPath(path);
+        if (url) {
+            args.source.path = url;
             logger.log(`Paths.setBP: Resolved ${path} to ${args.source.path}`);
             return true;
         } else {
@@ -80,8 +81,7 @@ export class UrlPathTransformer extends BasePathTransformer {
             if (frame.source.path) {
                 // Try to resolve the url to a path in the workspace. If it's not in the workspace,
                 // just use the script.url as-is. It will be resolved or cleared by the SourceMapTransformer.
-                const clientPath = this._targetUrlToClientPath.has(frame.source.path) ?
-                    this._targetUrlToClientPath.get(frame.source.path) :
+                const clientPath = this.getClientPathFromTargetPath(frame.source.path) ||
                     ChromeUtils.targetUrlToClientPath(this._webRoot, frame.source.path);
 
                 // Incoming stackFrames have sourceReference and path set. If the path was resolved to a file in the workspace,
@@ -93,4 +93,12 @@ export class UrlPathTransformer extends BasePathTransformer {
             }
         });
     }
+
+    public getTargetPathFromClientPath(clientPath: string): string {
+        return this._clientPathToTargetUrl.get(utils.canonicalizeUrl(clientPath));
+    }
+
+    public getClientPathFromTargetPath(targetPath: string): string {
+        return this._targetUrlToClientPath.get(utils.canonicalizeUrl(targetPath));
+    }
 }

test/chrome/chromeDebugAdapter.test.ts

@@ -111,7 +111,7 @@ suite('ChromeDebugAdapter', () => {
         const BP_ID = 'bpId';
         const FILE_NAME = 'file:///a.js';
         const SCRIPT_ID = '1';
-        function expectSetBreakpoint(breakpoints: DebugProtocol.SourceBreakpoint[], url: string, scriptId = SCRIPT_ID): void {
+        function expectSetBreakpoint(breakpoints: DebugProtocol.SourceBreakpoint[], url?: string, scriptId = SCRIPT_ID): void {
             breakpoints.forEach((bp, i) => {
                 const { line: lineNumber, column: columnNumber, condition } = bp;
 
@@ -161,6 +161,9 @@ suite('ChromeDebugAdapter', () => {
         }
 
         function emitScriptParsed(url = FILE_NAME, scriptId = SCRIPT_ID): void {
+            mockSourceMapTransformer.setup(m => m.scriptParsed(It.isValue(undefined), It.isValue(undefined)))
+                .returns(() => Promise.resolve([]));
+
             mockEventEmitter.emit('Debugger.scriptParsed', <Chrome.Debugger.Script>{ scriptId, url });
         }
 
@@ -282,7 +285,7 @@ suite('ChromeDebugAdapter', () => {
             const breakpoints: DebugProtocol.SourceBreakpoint[] = [
                 { line: 5, column: 6 }
             ];
-            expectSetBreakpoint(breakpoints, '');
+            expectSetBreakpoint(breakpoints);
 
             return chromeDebugAdapter.attach(ATTACH_ARGS)
                 .then(() => emitScriptParsed(/*url=*/'', SCRIPT_ID))
@@ -337,10 +340,13 @@ suite('ChromeDebugAdapter', () => {
             let scriptParsedFired = false;
             return chromeDebugAdapter.attach(ATTACH_ARGS).then(() => {
                 mockPathTransformer.setup(m => m.scriptParsed(It.isAnyString()))
-                    .callback(url => {
+                    .returns(url => {
                         scriptParsedFired = true;
                         assert(!!url); // Should be called with some default url
+                        return url;
                     });
+                mockSourceMapTransformer.setup(m => m.scriptParsed(It.isAny(), It.isValue(undefined)))
+                    .returns(() => Promise.resolve([]));
 
                 emitScriptParsed(/*url=*/'');
                 assert(scriptParsedFired);