Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Commit e56a4c7

Browse files
committed
fix #13 'Report unverified breakpoints back to user'
1 parent 28b4541 commit e56a4c7

File tree

1 file changed

+94
-56
lines changed

1 file changed

+94
-56
lines changed

src/node/nodeDebug.ts

Lines changed: 94 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,27 @@ export class ArrayExpander implements Expandable {
8080
}
8181
}
8282

83+
/**
84+
* This class represents an internal line/column breakpoint.
85+
*/
86+
export class InternalBreakpoint {
87+
88+
/** The requested breakpoint line location in the JavaScript file. */
89+
line: number;
90+
/** The requested breakpoint column location in the JavaScript file. */
91+
column: number;
92+
93+
/** do not try to set this breakpoint (because source locations could not successfully be mapped to destination locations) */
94+
ignore: boolean;
95+
96+
/** true if this breakpoint could be set and node.js returned an 'actual location' for it. */
97+
verified: boolean;
98+
/** The actual line location of this breakpoint in client coordinates. Contains the original client line if breakpoint could not be verified. */
99+
actualLine: number;
100+
/** the actual column location of this breakpoint in client coordinates. Contains the original client column if breakpoint could not be verified. */
101+
actualColumn: number;
102+
}
103+
83104
/**
84105
* This interface should always match the schema found in the node-debug extension manifest.
85106
*/
@@ -760,11 +781,16 @@ export class NodeDebugSession extends DebugSession {
760781
const clientLines = args.lines;
761782

762783
// convert line numbers from client
763-
const lines = new Array<number>(clientLines.length);
764-
const columns = new Array<number>(clientLines.length);
784+
const lbs = new Array<InternalBreakpoint>(clientLines.length);
765785
for (let i = 0; i < clientLines.length; i++) {
766-
lines[i] = this.convertClientLineToDebugger(clientLines[i]);
767-
columns[i] = 0;
786+
lbs[i] = {
787+
actualLine: clientLines[i],
788+
actualColumn: this.convertDebuggerColumnToClient(1), // hardcoded for now
789+
line: this.convertClientLineToDebugger(clientLines[i]),
790+
column: 0, // hardcoded for now
791+
verified: false,
792+
ignore: false
793+
};
768794
}
769795

770796
let scriptId = -1;
@@ -783,13 +809,16 @@ export class NodeDebugSession extends DebugSession {
783809
if (p) {
784810
sourcemap = true;
785811
// source map line numbers
786-
for (let i = 0; i < lines.length; i++) {
812+
for (let i = 0; i < lbs.length; i++) {
787813
let pp = path;
788-
const mr = this._sourceMaps.MapFromSource(pp, lines[i], columns[i]);
814+
const mr = this._sourceMaps.MapFromSource(pp, lbs[i].line, lbs[i].column);
789815
if (mr) {
790816
pp = mr.path;
791-
lines[i] = mr.line;
792-
columns[i] = mr.column;
817+
lbs[i].line = mr.line;
818+
lbs[i].column = mr.column;
819+
} else {
820+
// we couldn't map this breakpoint -> do not try to set it
821+
lbs[i].ignore = true;
793822
}
794823
if (pp !== p) {
795824
// console.error(`setBreakPointsRequest: sourceMap limitation ${pp}`);
@@ -798,26 +827,20 @@ export class NodeDebugSession extends DebugSession {
798827
path = p;
799828
}
800829
else if (!NodeDebugSession.isJavaScript(path)) {
801-
// return these breakpoints as unverified
802-
const bpts = new Array<Breakpoint>();
803-
for (let l of clientLines) {
804-
bpts.push(new Breakpoint(false, l));
830+
// ignore all breakpoints for this source
831+
for (let lb of lbs) {
832+
lb.ignore = true;
805833
}
806-
response.body = {
807-
breakpoints: bpts
808-
};
809-
this.sendResponse(response);
810-
return;
811834
}
812-
this._clearAllBreakpoints(response, path, -1, lines, columns, sourcemap, clientLines);
835+
this._clearAllBreakpoints(response, path, -1, lbs, sourcemap);
813836
return;
814837
}
815838

816839
if (source.name) {
817840
this.findModule(source.name, (id: number) => {
818841
if (id >= 0) {
819842
scriptId = id;
820-
this._clearAllBreakpoints(response, null, scriptId, lines, columns, sourcemap, clientLines);
843+
this._clearAllBreakpoints(response, null, scriptId, lbs, sourcemap);
821844
return;
822845
} else {
823846
this.sendErrorResponse(response, 2019, "internal module {_module} not found", { _module: source.name });
@@ -829,7 +852,7 @@ export class NodeDebugSession extends DebugSession {
829852

830853
if (source.sourceReference > 0) {
831854
scriptId = source.sourceReference - 1000;
832-
this._clearAllBreakpoints(response, null, scriptId, lines, columns, sourcemap, clientLines);
855+
this._clearAllBreakpoints(response, null, scriptId, lbs, sourcemap);
833856
return;
834857
}
835858

@@ -839,7 +862,7 @@ export class NodeDebugSession extends DebugSession {
839862
/*
840863
* Phase 2 of setBreakpointsRequest: clear all breakpoints of a given file
841864
*/
842-
private _clearAllBreakpoints(response: DebugProtocol.SetBreakpointsResponse, path: string, scriptId: number, lines: number[], columns: number[], sourcemap: boolean, clientLines: number[]): void {
865+
private _clearAllBreakpoints(response: DebugProtocol.SetBreakpointsResponse, path: string, scriptId: number, lbs: InternalBreakpoint[], sourcemap: boolean): void {
843866

844867
// clear all existing breakpoints for the given path or script ID
845868
this._node.command('listbreakpoints', null, (nodeResponse: NodeV8Response) => {
@@ -867,7 +890,7 @@ export class NodeDebugSession extends DebugSession {
867890
}
868891

869892
this._clearBreakpoints(toClear, 0, () => {
870-
this._finishSetBreakpoints(response, path, scriptId, lines, columns, sourcemap, clientLines);
893+
this._finishSetBreakpoints(response, path, scriptId, lbs, sourcemap);
871894
});
872895

873896
} else {
@@ -906,11 +929,15 @@ export class NodeDebugSession extends DebugSession {
906929
/*
907930
* Finish the setBreakpointsRequest: set the breakpooints and send the verification response back to client
908931
*/
909-
private _finishSetBreakpoints(response: DebugProtocol.SetBreakpointsResponse, path: string, scriptId: number, lines: number[], columns: number[], sourcemap: boolean, clientLines: number[]): void {
932+
private _finishSetBreakpoints(response: DebugProtocol.SetBreakpointsResponse, path: string, scriptId: number, lbs: InternalBreakpoint[], sourcemap: boolean): void {
910933

911-
const breakpoints = new Array<Breakpoint>();
934+
this._setBreakpoints(0, path, scriptId, lbs, sourcemap, () => {
935+
936+
const breakpoints = new Array<Breakpoint>();
937+
for (let lb of lbs) {
938+
breakpoints.push(new Breakpoint(lb.verified, lb.actualLine /* ; lb.clientColumn */));
939+
}
912940

913-
this._setBreakpoints(breakpoints, 0, path, scriptId, lines, columns, sourcemap, clientLines, () => {
914941
response.body = {
915942
breakpoints: breakpoints
916943
};
@@ -921,45 +948,47 @@ export class NodeDebugSession extends DebugSession {
921948
/**
922949
* Recursive function for setting node breakpoints.
923950
*/
924-
private _setBreakpoints(breakpoints: Array<Breakpoint>, ix: number, path: string, scriptId: number, lines: number[], columns: number[], sourcemap: boolean, clientLines: number[], done: () => void) : void {
951+
private _setBreakpoints(ix: number, path: string, scriptId: number, lbs: InternalBreakpoint[], sourcemap: boolean, done: () => void) : void {
925952

926-
if (lines.length == 0) { // nothing to do
953+
if (lbs.length == 0) { // nothing to do
927954
done();
928955
return;
929956
}
930957

931-
this._robustSetBreakPoint(scriptId, path, lines[ix], columns[ix], (verified: boolean, actualLine, actualColumn) => {
958+
this._robustSetBreakPoint(scriptId, path, lbs[ix], (success: boolean, actualLine: number, actualColumn: number) => {
932959

933-
// prepare sending breakpoint locations back to client
934-
let sourceLine = clientLines[ix]; // we start with the original lines from the client
960+
if (success) {
961+
// breakpoint successfully set and we've got an actual location
935962

936-
if (verified) {
937963
if (sourcemap) {
938-
if (!this._lazy) { // only if not in lazy mode we try to map actual Positions back
964+
// this source uses a sourcemap so we have to map locations back
965+
966+
if (!this._lazy) { // only if not in lazy mode we try to map actual positions back
939967
// map adjusted js breakpoints back to source language
940968
if (path && this._sourceMaps) {
941-
const p = path;
942-
const mr = this._sourceMaps.MapToSource(p, actualLine, actualColumn);
969+
const mr = this._sourceMaps.MapToSource(path, actualLine, actualColumn);
943970
if (mr) {
944971
actualLine = mr.line;
945972
actualColumn = mr.column;
946973
}
947974
}
948-
sourceLine = this.convertDebuggerLineToClient(actualLine);
975+
lbs[ix].actualLine = this.convertDebuggerLineToClient(actualLine);
949976
}
950977
} else {
951-
sourceLine = this.convertDebuggerLineToClient(actualLine);
978+
lbs[ix].actualLine = this.convertDebuggerLineToClient(actualLine);
952979
}
980+
953981
}
954-
breakpoints[ix] = new Breakpoint(verified, sourceLine);
982+
lbs[ix].verified = success;
955983

956984
// nasty corner case: since we ignore the break-on-entry event we have to make sure that we
957985
// stop in the entry point line if the user has an explicit breakpoint there.
958986
// For this we check here whether a breakpoint is at the same location as the "break-on-entry" location.
959987
// If yes, then we plan for hitting the breakpoint instead of "continue" over it!
988+
960989
if (!this._stopOnEntry) { // only relevant if we do not stop on entry
961-
const li = verified ? actualLine : lines[ix];
962-
const co = columns[ix]; // verified ? actualColumn : columns[ix];
990+
const li = success ? actualLine : lbs[ix].line;
991+
const co = lbs[ix].column; // success ? actualColumn : columns[ix];
963992
if (this._entryPath === path && this._entryLine === li && this._entryColumn === co) {
964993
// if yes, we do not have to "continue" but we have to generate a stopped event instead
965994
this._needContinue = false;
@@ -968,10 +997,10 @@ export class NodeDebugSession extends DebugSession {
968997
}
969998
}
970999

971-
if (ix+1 < lines.length) {
1000+
if (ix+1 < lbs.length) {
9721001
setImmediate(() => {
9731002
// recurse
974-
this._setBreakpoints(breakpoints, ix+1, path, scriptId, lines, columns, sourcemap, clientLines, done);
1003+
this._setBreakpoints(ix+1, path, scriptId, lbs, sourcemap, done);
9751004
});
9761005
} else {
9771006
done();
@@ -980,21 +1009,30 @@ export class NodeDebugSession extends DebugSession {
9801009
}
9811010

9821011
/*
983-
* register a single breakpoint with node and retry if it fails due to drive letter casing (on Windows)
1012+
* register a single breakpoint with node and retry if it fails due to drive letter casing (on Windows).
1013+
* On success the actual line and column is returned.
9841014
*/
985-
private _robustSetBreakPoint(scriptId: number, path: string, l: number, c: number, done: (success: boolean, actualLine?: number, actualColumn?: number) => void): void {
986-
this._setBreakpoint(scriptId, path, l, c, (verified: boolean, actualLine, actualColumn) => {
987-
if (verified) {
1015+
private _robustSetBreakPoint(scriptId: number, path: string, lb: InternalBreakpoint, done: (success: boolean, actualLine?: number, actualColumn?: number) => void): void {
1016+
1017+
if (lb.ignore) {
1018+
// ignore this breakpoint because it couldn't be source mapped successfully
1019+
done(false);
1020+
return;
1021+
}
1022+
1023+
this._setBreakpoint(scriptId, path, lb, (success: boolean, actualLine: number, actualColumn: number) => {
1024+
1025+
if (success) {
9881026
done(true, actualLine, actualColumn);
9891027
return;
9901028
}
9911029

992-
// take care of a mismatch of drive letter caseing
1030+
// failure -> guess: mismatch of drive letter caseing
9931031
const root = PathUtils.getPathRoot(path);
9941032
if (root && root.length === 3) { // root contains a drive letter
9951033
path = path.substring(0, 1).toUpperCase() + path.substring(1);
996-
this._setBreakpoint(scriptId, path, l, c, (verified: boolean, actualLine, actualColumn) => {
997-
if (verified) {
1034+
this._setBreakpoint(scriptId, path, lb, (success: boolean, actualLine, actualColumn) => {
1035+
if (success) {
9981036
done(true, actualLine, actualColumn);
9991037
} else {
10001038
done(false);
@@ -1009,20 +1047,20 @@ export class NodeDebugSession extends DebugSession {
10091047
/*
10101048
* register a single breakpoint with node.
10111049
*/
1012-
private _setBreakpoint(scriptId: number, path: string, l: number, c: number, cb: (success: boolean, actualLine?: number, actualColumn?: number) => void): void {
1050+
private _setBreakpoint(scriptId: number, path: string, lb: InternalBreakpoint, cb: (success: boolean, actualLine?: number, actualColumn?: number) => void): void {
10131051

1014-
if (l === 0) {
1015-
c += NodeDebugSession.FIRST_LINE_OFFSET;
1052+
if (lb.line === 0) {
1053+
lb.column += NodeDebugSession.FIRST_LINE_OFFSET;
10161054
}
10171055

1018-
let actualLine = l;
1019-
let actualColumn = c;
1056+
let actualLine = lb.line;
1057+
let actualColumn = lb.column;
10201058

10211059
let a: any;
10221060
if (scriptId > 0) {
1023-
a = { type: 'scriptId', target: scriptId, line: l, column: c };
1061+
a = { type: 'scriptId', target: scriptId, line: lb.line, column: lb.column };
10241062
} else {
1025-
a = { type: 'script', target: path, line: l, column: c };
1063+
a = { type: 'script', target: path, line: lb.line, column: lb.column };
10261064
}
10271065

10281066
this._node.command('setbreakpoint', a, (resp: NodeV8Response) => {
@@ -1038,7 +1076,7 @@ export class NodeDebugSession extends DebugSession {
10381076
actualColumn = 0;
10391077
}
10401078

1041-
if (actualLine !== l) {
1079+
if (actualLine !== lb.line) {
10421080
// console.error(`setbreakpoint: ${l} !== ${actualLine}`);
10431081
}
10441082
cb(true, actualLine, actualColumn);

0 commit comments

Comments
 (0)