Skip to content

Commit 288c28f

Browse files
committed
Send errors as evaluationErrors instead of diagnostics
1 parent e62b964 commit 288c28f

File tree

2 files changed

+32
-42
lines changed

2 files changed

+32
-42
lines changed

integration-tests/debugger/snapshot-time-budget.spec.js

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -38,38 +38,27 @@ describe('Dynamic Instrumentation', function () {
3838
'should timeout first, then disable subsequent snapshots and emit error diagnostics',
3939
async function () {
4040
const breakpoint = t.breakpoints[1]
41+
const expectedEvaluationErrors = [{
42+
expr: '',
43+
message: 'An object with 1000000 properties was detected while collecting a snapshot. This exceeds ' +
44+
`the maximum number of allowed properties of ${LARGE_OBJECT_SKIP_THRESHOLD}. Future snapshots for ` +
45+
'existing probes in this location will be skipped until the Node.js process is restarted'
46+
}]
4147

4248
// Listen for the first snapshot payload (should contain notCapturedReason: "timeout")
43-
const firstPayloadReceived = new Promise((resolve) => {
44-
t.agent.once('debugger-input', ({ payload: [{ debugger: { snapshot: { captures } } }] }) => {
45-
const { locals } = captures.lines[breakpoint.line]
46-
resolve(locals)
49+
const firstPayloadReceived = new Promise(/** @type {() => void} */ (resolve) => {
50+
t.agent.once('debugger-input', ({ payload: [{ debugger: { snapshot } }] }) => {
51+
const { locals } = snapshot.captures.lines[breakpoint.line]
52+
assert.strictEqual(
53+
containsTimeBudget(locals),
54+
true,
55+
'expected at least one field/element to be marked with notCapturedReason: "timeout"'
56+
)
57+
assert.deepStrictEqual(snapshot.evaluationErrors, expectedEvaluationErrors)
58+
resolve()
4759
})
4860
})
4961

50-
// Prepare to assert that an ERROR diagnostics event with exception details is emitted
51-
const errorDiagnosticsReceived = new Promise((/** @type {(value?: unknown) => void} */ resolve, reject) => {
52-
const handler = ({ payload }) => {
53-
payload.forEach(({ debugger: { diagnostics } }) => {
54-
if (diagnostics.status !== 'ERROR') return
55-
try {
56-
assert.strictEqual(
57-
diagnostics.exception.message,
58-
'An object with 1000000 properties was detected while collecting a snapshot. This exceeds the ' +
59-
'maximum number of allowed properties of 500. Future snapshots for existing probes in this ' +
60-
'location will be skipped until the Node.js process is restarted'
61-
)
62-
resolve()
63-
} catch (e) {
64-
reject(e)
65-
} finally {
66-
t.agent.off('debugger-diagnostics', handler)
67-
}
68-
})
69-
}
70-
t.agent.on('debugger-diagnostics', handler)
71-
})
72-
7362
// Install probe with snapshot capture enabled
7463
t.agent.addRemoteConfig(breakpoint.generateRemoteConfig({
7564
captureSnapshot: true,
@@ -82,18 +71,14 @@ describe('Dynamic Instrumentation', function () {
8271
result1.data.paused >= 1_000,
8372
`expected thread to be paused for at least 1 second, but was paused for ~${result1.data.paused}ms`
8473
)
85-
const locals = await firstPayloadReceived
86-
assert.strictEqual(
87-
containsTimeBudget(locals),
88-
true,
89-
'expected at least one field/element to be marked with notCapturedReason: "timeout"'
90-
)
91-
await errorDiagnosticsReceived
74+
75+
await firstPayloadReceived
9276

9377
// Prepare to assert that no snapshot is produced on a subsequent trigger
94-
const noSnapshotAfterSecondTrigger = new Promise((/** @type {(value?: unknown) => void} */ resolve) => {
95-
t.agent.once('debugger-input', ({ payload: [{ debugger: { snapshot: { captures } } }] }) => {
96-
assert.strictEqual(captures, undefined)
78+
const secondPayloadReceived = new Promise(/** @type {() => void} */ (resolve) => {
79+
t.agent.once('debugger-input', ({ payload: [{ debugger: { snapshot } }] }) => {
80+
assert.ok(!Object.hasOwn(snapshot, 'captures'))
81+
assert.deepStrictEqual(snapshot.evaluationErrors, expectedEvaluationErrors)
9782
resolve()
9883
})
9984
})
@@ -105,7 +90,7 @@ describe('Dynamic Instrumentation', function () {
10590
`expected thread to be paused <=5ms, but was paused for ~${result2.data.paused}ms`
10691
)
10792

108-
await noSnapshotAfterSecondTrigger
93+
await secondPayloadReceived
10994
}
11095
)
11196
})

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const session = require('./session')
66
const { getLocalStateForCallFrame } = require('./snapshot')
77
const send = require('./send')
88
const { getStackFromCallFrames } = require('./state')
9-
const { ackEmitting, ackError } = require('./status')
9+
const { ackEmitting } = require('./status')
1010
const config = require('./config')
1111
const { MAX_SNAPSHOTS_PER_SECOND_GLOBALLY } = require('./defaults')
1212
const log = require('./log')
@@ -219,15 +219,20 @@ session.on('Debugger.paused', async ({ params }) => {
219219
if (captureErrors.length > 0) {
220220
// There was an error collecting the snapshot for this probe, let's not try again
221221
probe.captureSnapshot = false
222-
for (const error of captureErrors) {
223-
ackError(error, probe)
224-
}
222+
probe.permanentEvaluationErrors = captureErrors.map(error => ({
223+
expr: '',
224+
message: error.message
225+
}))
225226
}
226227
snapshot.captures = {
227228
lines: { [probe.location.lines[0]]: { locals: processLocalState() } }
228229
}
229230
}
230231

232+
if (probe.permanentEvaluationErrors !== undefined) {
233+
snapshot.evaluationErrors = [...probe.permanentEvaluationErrors]
234+
}
235+
231236
let message = ''
232237
if (probe.templateRequiresEvaluation) {
233238
const results = evalResults[messageIndex++]

0 commit comments

Comments
 (0)