Skip to content

Commit 4c15f12

Browse files
authored
fix(debugger): abort on unknown Debugger.paused reason (#6901)
When the `Debugger.paused` event is emitted in the debugger implementation, it's expected to be because a breakpoint is hit. In that case the events `params.reason` will be set to `other`. However, if the inspected thread experiences an OOM event, `Debugger.paused` will also be emitted with `params.reason` set to `OOM`. There might also be other situations where `params.reason` is different from `other`. In all these situations we should abort and close the debugger. In the case of an OOM event in the inspected thread, exiting the debugging thread, will also stop the debugging session, and this will allow the inspected thread to properly exit with an OOM error. If we didn't do this we would essentially swallow the OOM error.
1 parent 72cd9aa commit 4c15f12

File tree

2 files changed

+36
-1
lines changed

2 files changed

+36
-1
lines changed

packages/dd-trace/src/debugger/devtools_client/index.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ if (SUPPORT_ARRAY_BUFFER_RESIZE) {
5858
session.on('Debugger.paused', async ({ params }) => {
5959
const start = process.hrtime.bigint()
6060

61+
if (params.reason !== 'other') {
62+
// This error should not be caught, and should exit the worker thread, effectively stopping the debugging session
63+
throw new Error(`Unexpected Debugger.paused reason: ${params.reason}`)
64+
}
65+
6166
let maxReferenceDepth, maxCollectionSize, maxFieldCount, maxLength
6267
let sampled = false
6368
let numberOfProbesWithSnapshots = 0

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,22 @@ const functionName = 'myFn'
1616
const parentThreadId = 'my-parent-thread-id'
1717
const event = {
1818
params: {
19+
reason: 'other',
1920
hitBreakpoints: [breakpointId],
2021
callFrames: [{ functionName, location: { scriptId, lineNumber: breakpoint.line - 1, columnNumber: 0 } }]
2122
}
2223
}
2324

2425
describe('onPause', function () {
25-
let session, send, onPaused, ackReceived
26+
let session, send, onPaused, ackReceived, log
2627

2728
beforeEach(async function () {
2829
ackReceived = sinon.spy()
30+
log = {
31+
error: sinon.spy(),
32+
debug: sinon.spy(),
33+
'@noCallThru': true
34+
}
2935

3036
session = {
3137
on: sinon.spy((event, listener) => {
@@ -68,6 +74,7 @@ describe('onPause', function () {
6874
'./session': session,
6975
'./state': state,
7076
'./snapshot': snapshot,
77+
'./log': log,
7178
'./send': send,
7279
'./status': { ackReceived },
7380
'./remote_config': { '@noCallThru': true }
@@ -83,4 +90,27 @@ describe('onPause', function () {
8390
expect(ackReceived).to.not.have.been.called
8491
expect(send).to.not.have.been.called
8592
})
93+
94+
it('should throw if paused for an unknown reason', async function () {
95+
const unknownReasonEvent = {
96+
...event,
97+
params: {
98+
...event.params,
99+
reason: 'OOM'
100+
}
101+
}
102+
103+
let thrown
104+
try {
105+
await onPaused(unknownReasonEvent)
106+
} catch (err) {
107+
thrown = err
108+
}
109+
110+
expect(thrown).to.be.an('error')
111+
expect(thrown.message).to.equal('Unexpected Debugger.paused reason: OOM')
112+
expect(session.post).to.not.have.been.called
113+
expect(ackReceived).to.not.have.been.called
114+
expect(send).to.not.have.been.called
115+
})
86116
})

0 commit comments

Comments
 (0)