Skip to content

Commit 6dce963

Browse files
watsonBridgeAR
authored andcommitted
test(debugger): cover breakpoint error paths (#8996)
Add focused unit coverage for existing debugger breakpoint behavior that was previously untested. The new cases cover source map location translation failures, inspector breakpoint operation failures, and async re-evaluation error logging.
1 parent d1e4732 commit 6dce963

1 file changed

Lines changed: 134 additions & 2 deletions

File tree

packages/dd-trace/test/debugger/devtools_client/breakpoints.spec.js

Lines changed: 134 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ describe('breakpoints', function () {
2323
* }}
2424
*/
2525
let sessionMock
26+
/**
27+
* @type {{
28+
* getGeneratedPosition: sinon.SinonStub;
29+
* '@noCallThru': boolean;
30+
* }}
31+
*/
32+
let sourceMapsMock
2633
/**
2734
* @type {{
2835
* debug: sinon.SinonStub;
@@ -31,6 +38,8 @@ describe('breakpoints', function () {
3138
* }}
3239
*/
3340
let logMock
41+
/** @type {() => void} */
42+
let scriptLoadingStabilizedCallback
3443
/**
3544
* @type {{
3645
* findScriptFromPartialPath: sinon.SinonStub;
@@ -77,14 +86,22 @@ describe('breakpoints', function () {
7786
}),
7887
/**
7988
* @param {string} event
80-
* @param {Function} callback
89+
* @param {() => void} callback
8190
*/
8291
on (event, callback) {
83-
if (event === 'scriptLoadingStabilized') callback()
92+
if (event === 'scriptLoadingStabilized') {
93+
scriptLoadingStabilizedCallback = callback
94+
callback()
95+
}
8496
},
8597
'@noCallThru': true,
8698
}
8799

100+
sourceMapsMock = {
101+
getGeneratedPosition: sinon.stub(),
102+
'@noCallThru': true,
103+
}
104+
88105
logMock = {
89106
debug: sinon.stub(),
90107
error: sinon.stub(),
@@ -108,6 +125,7 @@ describe('breakpoints', function () {
108125

109126
breakpoints = proxyquire('../../../src/debugger/devtools_client/breakpoints', {
110127
'./session': sessionMock,
128+
'./source-maps': sourceMapsMock,
111129
'./state': stateMock,
112130
'./log': logMock,
113131
})
@@ -176,6 +194,51 @@ describe('breakpoints', function () {
176194
)
177195
})
178196

197+
it('should translate source-mapped locations before setting the breakpoint', async function () {
198+
stateMock.findScriptFromPartialPath.returns({
199+
url: 'file:///path/to/test.js',
200+
scriptId: 'script-1',
201+
sourceMapURL: 'test.js.map',
202+
source: 'source',
203+
})
204+
sourceMapsMock.getGeneratedPosition.resolves({ line: 12, column: 4 })
205+
206+
await addProbe()
207+
208+
sinon.assert.calledOnceWithExactly(
209+
sourceMapsMock.getGeneratedPosition,
210+
'file:///path/to/test.js',
211+
'source',
212+
10,
213+
'test.js.map'
214+
)
215+
sinon.assert.calledWith(sessionMock.post.secondCall, 'Debugger.setBreakpoint', {
216+
location: {
217+
scriptId: 'script-1',
218+
lineNumber: 11,
219+
columnNumber: 4,
220+
},
221+
condition: compileBreakpointCondition([{ id: 'probe-1', samplingIndex: 0, nsBetweenSampling: 200000n }]),
222+
})
223+
})
224+
225+
it('should throw if a source map cannot resolve the generated location', async function () {
226+
stateMock.findScriptFromPartialPath.returns({
227+
url: 'file:///path/to/test.js',
228+
scriptId: 'script-1',
229+
sourceMapURL: 'test.js.map',
230+
source: 'source',
231+
})
232+
sourceMapsMock.getGeneratedPosition.resolves({ line: null, column: null })
233+
234+
await assert.rejects(
235+
addProbe(),
236+
{
237+
message: 'Could not find generated position for file:///path/to/test.js:10:0 (probe: probe-1, version: 1)',
238+
}
239+
)
240+
})
241+
179242
describe('capture limits', function () {
180243
it('should set default capture limits when captureSnapshot is true', async function () {
181244
await addProbe({ captureSnapshot: true })
@@ -468,6 +531,26 @@ describe('breakpoints', function () {
468531
})
469532
})
470533

534+
it('should wrap errors when setting a new breakpoint fails', async function () {
535+
const cause = new Error('inspector failure')
536+
sessionMock.post.callsFake((method, { location } = {}) => {
537+
if (method === 'Debugger.setBreakpoint') {
538+
return Promise.reject(cause)
539+
}
540+
return Promise.resolve({})
541+
})
542+
543+
await assert.rejects(
544+
addProbe(),
545+
(err) => {
546+
assert(err instanceof Error)
547+
assert.strictEqual(err.message, 'Error setting breakpoint for probe probe-1 (version: 1)')
548+
assert.strictEqual(err.cause, cause)
549+
return true
550+
}
551+
)
552+
})
553+
471554
it('should wrap errors when replacing a breakpoint while adding a probe fails', async function () {
472555
await addProbe()
473556
sessionMock.post.resetHistory()
@@ -942,6 +1025,35 @@ describe('breakpoints', function () {
9421025
})
9431026
})
9441027

1028+
it('should wrap errors when removing a breakpoint fails', async function () {
1029+
await addProbe()
1030+
await addProbe({ id: 'probe-2', where: { sourceFile: 'test2.js', lines: ['20'] } })
1031+
sessionMock.post.resetHistory()
1032+
1033+
const cause = new Error('inspector failure')
1034+
sessionMock.post.callsFake((method, { location } = {}) => {
1035+
if (method === 'Debugger.removeBreakpoint') {
1036+
return Promise.reject(cause)
1037+
}
1038+
if (method === 'Debugger.setBreakpoint') {
1039+
return Promise.resolve({
1040+
breakpointId: `bp-${location.scriptId}:${location.lineNumber}:${location.columnNumber}`,
1041+
})
1042+
}
1043+
return Promise.resolve({})
1044+
})
1045+
1046+
await assert.rejects(
1047+
breakpoints.removeBreakpoint({ id: 'probe-1' }),
1048+
(err) => {
1049+
assert(err instanceof Error)
1050+
assert.strictEqual(err.message, 'Error removing breakpoint for probe probe-1')
1051+
assert.strictEqual(err.cause, cause)
1052+
return true
1053+
}
1054+
)
1055+
})
1056+
9451057
it('should throw error if debugger not started', async function () {
9461058
await breakpoints.removeBreakpoint({ id: 'probe-1' })
9471059
.then(() => {
@@ -1032,6 +1144,26 @@ describe('breakpoints', function () {
10321144
})
10331145
})
10341146

1147+
describe('re-evaluation', function () {
1148+
it('should log errors from async probe re-evaluation', async function () {
1149+
await addProbe()
1150+
logMock.error.resetHistory()
1151+
1152+
const cause = new Error('script lookup failure')
1153+
stateMock.findScriptFromPartialPath.throws(cause)
1154+
1155+
scriptLoadingStabilizedCallback()
1156+
await Promise.resolve()
1157+
1158+
sinon.assert.calledWith(
1159+
logMock.error,
1160+
'[debugger:devtools_client] Error re-evaluating probe %s',
1161+
'probe-1',
1162+
cause
1163+
)
1164+
})
1165+
})
1166+
10351167
/**
10361168
* Add a generated probe.
10371169
*

0 commit comments

Comments
 (0)