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

Commit 7a7b20d

Browse files
gaofanmichaelgaofanmichael
authored
Adjust api breakpoint if resolved in another line (#330)
* Adjust api breakpoint if resolved in another line Change api type breakpoint location line number if the breakpoint was originally set to a line with only comments. The new line that breakpoint set to will be actual paused line in code.
1 parent bd4a59c commit 7a7b20d

File tree

4 files changed

+59
-34
lines changed

4 files changed

+59
-34
lines changed

‎src/agent/state.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ class StateResolver {
169169
// evaluated expressions can be evaluated as much as possible within
170170
// the max data size limits
171171
const frames = that.resolveFrames_();
172-
173172
// Now resolve the variables
174173
let index = this.messageTable_.length; // skip the sentinel values
175174
const noLimit = that.config_.capture.maxDataSize === 0;
@@ -190,8 +189,11 @@ class StateResolver {
190189
if (index < that.rawVariableTable_.length) {
191190
that.trimVariableTable_(index, frames);
192191
}
193-
194192
return {
193+
// TODO (fgao): Add path attribute to avoid explicit cast to
194+
// apiTypes.SourceLocation once breakpoint is passed in this class.
195+
location: {line: this.state_.frame(0).sourceLine() + 1} as
196+
apiTypes.SourceLocation,
195197
stackFrames: frames,
196198
variableTable: that.resolvedVariableTable_,
197199
evaluatedExpressions: that.evaluatedExpressions_

‎src/agent/v8debugapi.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,9 @@ export function create(
612612
// TODO: Address the case where `breakpoint.expression` is `undefined`.
613613
const captured = state.capture(
614614
execState, breakpoint.expressions as string[], config, v8);
615+
if (breakpoint.location && captured.location && captured.location.line) {
616+
breakpoint.location.line = captured.location.line;
617+
}
615618
breakpoint.stackFrames = captured.stackFrames;
616619
// TODO: This suggests the Status type and Variable type are the same.
617620
// Determine if that is the case.

‎test/test-v8debugapi-code.js‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
/*1* KEEP THIS CODE AT THE TOP TO AVOID LINE NUMBER CHANGES */
22
/*2*/'use strict';
33
/*3*/function foo(n) {
4-
/*4*/ var A = [1, 2, 3]; var B = { a: 5, b: 6, c: 7 };
5-
/*5*/ return n+42+A[0]+B.b;
6-
/*6*/}
7-
/*7*/function getterObject() {
8-
/*8*/ var hasGetter = { _a: 5, get a() { return this._a; }, b: 'hello world' };
9-
/*9*/ return hasGetter.a;
10-
/*10*/}
11-
/*11*/module.exports = {
12-
/*12*/ foo: foo,
13-
/*13*/ getterObject: getterObject
14-
/*14*/};
4+
/*4*/ // some comments.
5+
/*5*/ var A = [1, 2, 3]; var B = { a: 5, b: 6, c: 7 };
6+
/*6*/ return n+42+A[0]+B.b;
7+
/*7*/}
8+
/*8*/function getterObject() {
9+
/*9*/ var hasGetter = { _a: 5, get a() { return this._a; }, b: 'hello world' };
10+
/*10*/ return hasGetter.a;
11+
/*11*/}
12+
/*12*/module.exports = {
13+
/*13*/ foo: foo,
14+
/*14*/ getterObject: getterObject
15+
/*15*/};

‎test/test-v8debugapi.ts‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {DebugAgentConfig} from '../src/agent/config';
2222
const breakpointInFoo: apiTypes.Breakpoint = {
2323
id: 'fake-id-123',
2424
// TODO: Determine if we should be restricting to only the build directory.
25-
location: { path: 'build/test/test-v8debugapi-code.js', line: 4 }
25+
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 }
2626
} as apiTypes.Breakpoint;
2727

2828
const MAX_INT = 2147483647; // Max signed int32.
@@ -394,22 +394,22 @@ describe('v8debugapi', function() {
394394
describe('path normalization', function() {
395395
// TODO: Have this actually be a list of Breakpoints
396396
const breakpoints = [
397-
{ id: 'path0', location: {line: 4, path: path.join(path.sep, 'test',
397+
{ id: 'path0', location: {line: 5, path: path.join(path.sep, 'test',
398398
'test-v8debugapi-code.js')}} as any as apiTypes.Breakpoint,
399-
{ id: 'path1', location: {line: 4, path: path.join('test',
399+
{ id: 'path1', location: {line: 5, path: path.join('test',
400400
'test-v8debugapi-code.js')}} as any as apiTypes.Breakpoint,
401-
{ id: 'path2', location: {line: 4, path:
401+
{ id: 'path2', location: {line: 5, path:
402402
// Usage the absolute path to `test-v8debugapi-code.js`.
403403
__filename.split(path.sep).slice(0, -1).concat('test-v8debugapi-code.js').join(path.sep)
404404
}} as any as apiTypes.Breakpoint,
405405
{ id: 'with . in path', location: {path: path.join('test', '.',
406-
'test-v8debugapi-code.js'), line: 4}} as any as apiTypes.Breakpoint,
406+
'test-v8debugapi-code.js'), line: 5}} as any as apiTypes.Breakpoint,
407407
{ id: 'with . in path', location: {path: path.join('.',
408-
'test-v8debugapi-code.js'), line: 4}} as any as apiTypes.Breakpoint,
408+
'test-v8debugapi-code.js'), line: 5}} as any as apiTypes.Breakpoint,
409409
{ id: 'with .. in path', location: {path: path.join('test', '..',
410-
'test-v8debugapi-code.js'), line: 4}} as any as apiTypes.Breakpoint,
410+
'test-v8debugapi-code.js'), line: 5}} as any as apiTypes.Breakpoint,
411411
{ id: 'with .. in path', location: {path: path.join('..', 'test',
412-
'test-v8debugapi-code.js'), line: 4}} as any as apiTypes.Breakpoint
412+
'test-v8debugapi-code.js'), line: 5}} as any as apiTypes.Breakpoint
413413
];
414414

415415
breakpoints.forEach(function(bp: apiTypes.Breakpoint) {
@@ -500,6 +500,25 @@ describe('v8debugapi', function() {
500500

501501
});
502502

503+
it('should resolve actual line number hit rather than originally set', function(done) {
504+
const bp: apiTypes.Breakpoint = {
505+
id: 'fake-id-124',
506+
location: { path: 'build/test/test-v8debugapi-code.js', line: 4 }
507+
} as any as apiTypes.Breakpoint;
508+
api.set(bp, function(err) {
509+
assert.ifError(err);
510+
api.wait(bp, function(err) {
511+
assert.ifError(err);
512+
assert.equal((bp.location as apiTypes.SourceLocation).line, 5);
513+
api.clear(bp, function(err) {
514+
assert.ifError(err);
515+
done();
516+
});
517+
})
518+
process.nextTick(function() {code.foo(1);});
519+
});
520+
});
521+
503522
it('should work with multiply hit breakpoints', function(done) {
504523
const oldWarn = logger.warn;
505524
let logCount = 0;
@@ -713,7 +732,7 @@ describe('v8debugapi', function() {
713732
id: 'fake-id-124',
714733
// TODO: This path can be lest strict when this file has been
715734
// converted to Typescript.
716-
location: { path: 'build/test/test-v8debugapi-code.js', line: 9 },
735+
location: { path: 'build/test/test-v8debugapi-code.js', line: 10 },
717736
expressions: ['process.env', 'hasGetter']
718737
} as any as apiTypes.Breakpoint;
719738
const oldMaxData = config.capture.maxDataSize;
@@ -767,7 +786,7 @@ describe('v8debugapi', function() {
767786
id: breakpointInFoo.id,
768787
// TODO: This path can be lest strict when this file has been
769788
// converted to Typescript.
770-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 },
789+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 },
771790
expressions: ['A']
772791
} as any as apiTypes.Breakpoint;
773792
api.set(bp, function(err) {
@@ -808,7 +827,7 @@ describe('v8debugapi', function() {
808827
id: 'fake-id-124',
809828
// TODO: This path can be lest strict when this file has been
810829
// converted to Typescript.
811-
location: { path: 'build/test/test-v8debugapi-code.js', line: 9 }
830+
location: { path: 'build/test/test-v8debugapi-code.js', line: 10 }
812831
} as any as apiTypes.Breakpoint;
813832
const oldMaxLength = config.capture.maxStringLength;
814833
const oldMaxData = config.capture.maxDataSize;
@@ -851,7 +870,7 @@ describe('v8debugapi', function() {
851870
id: 'fake-id-124',
852871
// TODO: This path can be lest strict when this file has been
853872
// converted to Typescript.
854-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 }
873+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 }
855874
} as any as apiTypes.Breakpoint;
856875
const oldMax = config.capture.maxProperties;
857876
config.capture.maxProperties = 1;
@@ -887,7 +906,7 @@ describe('v8debugapi', function() {
887906
id: 'fake-id-124',
888907
// TODO: This path can be lest strict when this file has been
889908
// converted to Typescript.
890-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 }
909+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 }
891910
} as any as apiTypes.Breakpoint;
892911
const oldMax = config.capture.maxProperties;
893912
config.capture.maxProperties = 1;
@@ -923,7 +942,7 @@ describe('v8debugapi', function() {
923942
id: 'fake-id-124',
924943
// TODO: This path can be lest strict when this file has been
925944
// converted to Typescript.
926-
location: { path: 'build/test/test-v8debugapi-code.js', line: 9 },
945+
location: { path: 'build/test/test-v8debugapi-code.js', line: 10 },
927946
expressions: ['hasGetter']
928947
} as any as apiTypes.Breakpoint;
929948
const oldMaxLength = config.capture.maxStringLength;
@@ -965,7 +984,7 @@ describe('v8debugapi', function() {
965984
id: 'fake-id-124',
966985
// TODO: This path can be lest strict when this file has been
967986
// converted to Typescript.
968-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 },
987+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 },
969988
expressions: ['A']
970989
} as any as apiTypes.Breakpoint;
971990
const oldMaxProps = config.capture.maxProperties;
@@ -1004,7 +1023,7 @@ describe('v8debugapi', function() {
10041023
id: 'fake-id-124',
10051024
// TODO: This path can be lest strict when this file has been
10061025
// converted to Typescript.
1007-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 },
1026+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 },
10081027
expressions: ['B']
10091028
} as any as apiTypes.Breakpoint;
10101029
const oldMaxProps = config.capture.maxProperties;
@@ -1042,7 +1061,7 @@ describe('v8debugapi', function() {
10421061
id: 'fake-id-124',
10431062
// TODO: This path can be lest strict when this file has been
10441063
// converted to Typescript.
1045-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 },
1064+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 },
10461065
expressions: ['A']
10471066
} as any as apiTypes.Breakpoint;
10481067
const oldMaxProps = config.capture.maxProperties;
@@ -1081,7 +1100,7 @@ describe('v8debugapi', function() {
10811100
id: 'fake-id-124',
10821101
// TODO: This path can be lest strict when this file has been
10831102
// converted to Typescript.
1084-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 },
1103+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 },
10851104
expressions: ['B']
10861105
} as any as apiTypes.Breakpoint;
10871106
const oldMaxProps = config.capture.maxProperties;
@@ -1120,7 +1139,7 @@ describe('v8debugapi', function() {
11201139
id: 'fake-id-124',
11211140
// TODO: This path can be lest strict when this file has been
11221141
// converted to Typescript.
1123-
location: { path: 'build/test/test-v8debugapi-code.js', line: 5 },
1142+
location: { path: 'build/test/test-v8debugapi-code.js', line: 6 },
11241143
expressions: ['A']
11251144
} as any as apiTypes.Breakpoint;
11261145
const oldMaxProps = config.capture.maxProperties;
@@ -1399,9 +1418,9 @@ describe('v8debugapi', function() {
13991418
it('should be possible to set multiple breakpoints at once',
14001419
function(done) {
14011420
// TODO: Have this actually implement Breakpoint
1402-
const bp1: apiTypes.Breakpoint = { id: 'bp1', location: { path: __filename, line: 4 }} as any as apiTypes.Breakpoint;
1421+
const bp1: apiTypes.Breakpoint = { id: 'bp1', location: { path: __filename, line: 5 }} as any as apiTypes.Breakpoint;
14031422
// TODO: Have this actually implement Breakpoint
1404-
const bp2: apiTypes.Breakpoint = { id: 'bp2', location: { path: __filename, line: 5 }} as any as apiTypes.Breakpoint;
1423+
const bp2: apiTypes.Breakpoint = { id: 'bp2', location: { path: __filename, line: 6 }} as any as apiTypes.Breakpoint;
14051424
api.set(bp1, function(err) {
14061425
assert.ifError(err);
14071426
api.set(bp2, function(err) {

0 commit comments

Comments
 (0)