Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Commit

Permalink
Don't hang on to BP requests when the sourcemap can't be resolved-
Browse files Browse the repository at this point in the history 
Fix #38
  • Loading branch information
@roblourens
roblourens committed Sep 22, 2016
1 parent e464977 commit 4fa0289
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 171 deletions.
83 changes: 50 additions & 33 deletions src/chrome/chromeDebugAdapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,41 +369,58 @@ export abstract class ChromeDebugAdapter extends BaseDebugAdapter {

public setBreakpoints(args: ISetBreakpointsArgs, requestSeq: number): Promise<ISetBreakpointsResponseBody> {
this._lineNumberTransformer.setBreakpoints(args);
return this._sourceMapTransformer.setBreakpoints(args, requestSeq)
.then(() => this._pathTransformer.setBreakpoints(args))
.then(() => {
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;
}
}
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?).")));

canSetBp = this._pathTransformer.setBreakpoints(args);
if (!canSetBp) return Promise.resolve(this.unverifiedBpResponse(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;
}
}

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 utils.errP(`Can't find script for breakpoint request`);
}
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 utils.errP(`Can't find script for breakpoint request`);
}
}

private unverifiedBpResponse(args: ISetBreakpointsArgs, message?: string): ISetBreakpointsResponseBody {
const breakpoints = args.breakpoints.map(bp => {
return <DebugProtocol.Breakpoint>{
verified: false,
line: bp.line,
column: bp.column,
message
};
});

const response = { breakpoints };
this._lineNumberTransformer.setBreakpointsResponse(response);
return response;
}

public setFunctionBreakpoints(): Promise<any> {
Expand Down
4 changes: 2 additions & 2 deletions src/transformers/basePathTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export class BasePathTransformer {
return Promise.resolve<void>();
}

public setBreakpoints(args: ISetBreakpointsArgs): Promise<void> {
return Promise.resolve<void>();
public setBreakpoints(args: ISetBreakpointsArgs): boolean {
return true;
}

public clearTargetContext(): void {
Expand Down
155 changes: 60 additions & 95 deletions src/transformers/baseSourceMapTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@ import * as utils from '../utils';
import * as logger from '../logger';
import {ISourceContainer} from '../chrome/chromeDebugAdapter';

interface IPendingBreakpoint {
resolve: () => void;
reject: (e: Error) => void;
args: ISetBreakpointsArgs;
requestSeq: number;
}

/**
* If sourcemaps are enabled, converts from source files on the client side to runtime files on the target side
*/
Expand All @@ -30,7 +23,6 @@ export class BaseSourceMapTransformer {

private _requestSeqToSetBreakpointsArgs: Map<number, ISetBreakpointsArgs>;
private _allRuntimeScriptPaths: Set<string>;
private _pendingBreakpointsByPath = new Map<string, IPendingBreakpoint>();
private _authoredPathsToMappedBPs: Map<string, DebugProtocol.SourceBreakpoint[]>;

protected _preLoad = Promise.resolve<void>();
Expand Down Expand Up @@ -65,77 +57,73 @@ export class BaseSourceMapTransformer {
}

/**
* Apply sourcemapping to the setBreakpoints request path/lines
* Apply sourcemapping to the setBreakpoints request path/lines.
* Returns true if completed successfully, and setBreakpoint should continue.
*/
public setBreakpoints(args: ISetBreakpointsArgs, requestSeq: number): Promise<void> {
return new Promise<void>((resolve, reject) => {
if (!this._sourceMaps) {
resolve();
return;
}
public setBreakpoints(args: ISetBreakpointsArgs, requestSeq: number): boolean {
if (!this._sourceMaps) {
return true;
}

if (args.source.sourceReference) {
// If the source contents were inlined, then args.source has no path, but we
// stored it in the handle
const handle = this._sourceHandles.get(args.source.sourceReference);
if (handle.mappedPath) {
args.source.path = handle.mappedPath;
}
this._requestSeqToSetBreakpointsArgs.set(requestSeq, JSON.parse(JSON.stringify(args)));
if (args.source.sourceReference) {
// If the source contents were inlined, then args.source has no path, but we
// stored it in the handle
const handle = this._sourceHandles.get(args.source.sourceReference);
if (handle.mappedPath) {
args.source.path = handle.mappedPath;
}
}

if (args.source.path) {
const argsPath = args.source.path;
const mappedPath = this._sourceMaps.getGeneratedPathFromAuthoredPath(argsPath);
if (mappedPath) {
logger.log(`SourceMaps.setBP: Mapped ${argsPath} to ${mappedPath}`);
args.authoredPath = argsPath;
args.source.path = mappedPath;

// DebugProtocol doesn't send cols yet, but they need to be added from sourcemaps
args.breakpoints.forEach(bp => {
const { line, column = 0 } = bp;
const mapped = this._sourceMaps.mapToGenerated(argsPath, line, column);
if (mapped) {
logger.log(`SourceMaps.setBP: Mapped ${argsPath}:${line + 1}:${column + 1} to ${mappedPath}:${mapped.line + 1}:${mapped.column + 1}`);
bp.line = mapped.line;
bp.column = mapped.column;
} else {
logger.log(`SourceMaps.setBP: Mapped ${argsPath} but not line ${line + 1}, column 1`);
bp.column = column; // take 0 default if needed
}
});

this._authoredPathsToMappedBPs.set(argsPath, args.breakpoints);
if (args.source.path) {
const argsPath = args.source.path;
const mappedPath = this._sourceMaps.getGeneratedPathFromAuthoredPath(argsPath);
if (mappedPath) {
logger.log(`SourceMaps.setBP: Mapped ${argsPath} to ${mappedPath}`);
args.authoredPath = argsPath;
args.source.path = mappedPath;

// DebugProtocol doesn't send cols yet, but they need to be added from sourcemaps
args.breakpoints.forEach(bp => {
const { line, column = 0 } = bp;
const mapped = this._sourceMaps.mapToGenerated(argsPath, line, column);
if (mapped) {
logger.log(`SourceMaps.setBP: Mapped ${argsPath}:${line + 1}:${column + 1} to ${mappedPath}:${mapped.line + 1}:${mapped.column + 1}`);
bp.line = mapped.line;
bp.column = mapped.column;
} else {
logger.log(`SourceMaps.setBP: Mapped ${argsPath} but not line ${line + 1}, column 1`);
bp.column = column; // take 0 default if needed
}
});

// Include BPs from other files that map to the same file. Ensure the current file's breakpoints go first
this._sourceMaps.allMappedSources(mappedPath).forEach(sourcePath => {
if (sourcePath === argsPath) {
return;
}
this._authoredPathsToMappedBPs.set(argsPath, args.breakpoints);

const sourceBPs = this._authoredPathsToMappedBPs.get(sourcePath);
if (sourceBPs) {
// Don't modify the cached array
args.breakpoints = args.breakpoints.concat(sourceBPs);
}
});
} else if (this._allRuntimeScriptPaths.has(argsPath)) {
// It's a generated file which is loaded
logger.log(`SourceMaps.setBP: SourceMaps are enabled but ${argsPath} is a runtime script`);
} else {
// Source (or generated) file which is not loaded, need to wait
logger.log(`SourceMaps.setBP: ${argsPath} can't be resolved to a loaded script. It may just not be loaded yet.`);
this._pendingBreakpointsByPath.set(argsPath, { resolve, reject, args, requestSeq });
return;
}
// Include BPs from other files that map to the same file. Ensure the current file's breakpoints go first
this._sourceMaps.allMappedSources(mappedPath).forEach(sourcePath => {
if (sourcePath === argsPath) {
return;
}

this._requestSeqToSetBreakpointsArgs.set(requestSeq, JSON.parse(JSON.stringify(args)));
resolve();
const sourceBPs = this._authoredPathsToMappedBPs.get(sourcePath);
if (sourceBPs) {
// Don't modify the cached array
args.breakpoints = args.breakpoints.concat(sourceBPs);
}
});
} else if (this._allRuntimeScriptPaths.has(argsPath)) {
// It's a generated file which is loaded
logger.log(`SourceMaps.setBP: SourceMaps are enabled but ${argsPath} is a runtime script`);
} else {
// No source.path
resolve();
// Source (or generated) file which is not loaded, need to wait
logger.log(`SourceMaps.setBP: ${argsPath} can't be resolved to a loaded script. It may just not be loaded yet.`);
return false;
}
});
} else {
// No source.path
}

return true;
}

/**
Expand Down Expand Up @@ -214,20 +202,13 @@ export class BaseSourceMapTransformer {
if (this._sourceMaps) {
this._allRuntimeScriptPaths.add(pathToGenerated);

if (!sourceMapURL) {
// If a file does not have a source map, check if we've seen any breakpoints
// for it anyway and make sure to enable them
this.resolvePendingBreakpointsForScript(pathToGenerated);
return;
}
if (!sourceMapURL) return;

// Load the sourcemap for this new script and log its sources
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) }`);
sources.forEach(sourcePath => {
this.resolvePendingBreakpointsForScript(sourcePath);
});
}
});
}
Expand Down Expand Up @@ -255,20 +236,4 @@ export class BaseSourceMapTransformer {
public getGeneratedPathFromAuthoredPath(authoredPath: string): Promise<string> {
return this._preLoad.then(() => this._sourceMaps.getGeneratedPathFromAuthoredPath(authoredPath));
}

/**
* Resolve any pending breakpoints for this script
*/
private resolvePendingBreakpointsForScript(scriptUrl: string): void {
if (this._pendingBreakpointsByPath.has(scriptUrl)) {
logger.log(`SourceMaps.scriptParsed: Resolving pending breakpoints for ${scriptUrl}`);

let pendingBreakpoints = this._pendingBreakpointsByPath.get(scriptUrl);
this._pendingBreakpointsByPath.delete(scriptUrl);

// If there's a setBreakpoints request waiting on this script, go through setBreakpoints again
this.setBreakpoints(pendingBreakpoints.args, pendingBreakpoints.requestSeq)
.then(pendingBreakpoints.resolve, pendingBreakpoints.reject);
}
}
}
7 changes: 3 additions & 4 deletions src/transformers/remotePathTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,11 @@ export class RemotePathTransformer extends BasePathTransformer {
return localRootP;
}

public setBreakpoints(args: ISetBreakpointsArgs): Promise<void> {
if (!args.source.path) {
return Promise.resolve<void>();
public setBreakpoints(args: ISetBreakpointsArgs): boolean {
if (args.source.path) {
args.source.path = this.localToRemote(args.source.path);
}

args.source.path = this.localToRemote(args.source.path);
return super.setBreakpoints(args);
}

Expand Down
Loading

0 comments on commit 4fa0289

Please sign in to comment.