Skip to content

Commit a50d854

Browse files
watsonwatson
authored
Ensure the fake agent in integration tests doesn't swallow exceptions (#4995)
1 parent 1a95b0b commit a50d854

File tree

5 files changed

+49
-65
lines changed

5 files changed

+49
-65
lines changed

‎integration-tests/debugger/basic.spec.js

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const os = require('os')
44

55
const { assert } = require('chai')
66
const { pollInterval, setup } = require('./utils')
7-
const { assertObjectContains, assertUUID, failOnException } = require('../helpers')
7+
const { assertObjectContains, assertUUID } = require('../helpers')
88
const { ACKNOWLEDGED, ERROR } = require('../../packages/dd-trace/src/appsec/remote_config/apply_states')
99
const { version } = require('../../package.json')
1010

@@ -35,17 +35,17 @@ describe('Dynamic Instrumentation', function () {
3535
debugger: { diagnostics: { probeId, probeVersion: 0, status: 'EMITTING' } }
3636
}]
3737

38-
t.agent.on('remote-config-ack-update', failOnException(done, (id, version, state, error) => {
38+
t.agent.on('remote-config-ack-update', (id, version, state, error) => {
3939
assert.strictEqual(id, t.rcConfig.id)
4040
assert.strictEqual(version, 1)
4141
assert.strictEqual(state, ACKNOWLEDGED)
4242
assert.notOk(error) // falsy check since error will be an empty string, but that's an implementation detail
4343

4444
receivedAckUpdate = true
4545
endIfDone()
46-
}))
46+
})
4747

48-
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
48+
t.agent.on('debugger-diagnostics', ({ payload }) => {
4949
const expected = expectedPayloads.shift()
5050
assertObjectContains(payload, expected)
5151
assertUUID(payload.debugger.diagnostics.runtimeId)
@@ -60,7 +60,7 @@ describe('Dynamic Instrumentation', function () {
6060
} else {
6161
endIfDone()
6262
}
63-
}))
63+
})
6464

6565
t.agent.addRemoteConfig(t.rcConfig)
6666

@@ -97,22 +97,22 @@ describe('Dynamic Instrumentation', function () {
9797
() => {}
9898
]
9999

100-
t.agent.on('remote-config-ack-update', failOnException(done, (id, version, state, error) => {
100+
t.agent.on('remote-config-ack-update', (id, version, state, error) => {
101101
assert.strictEqual(id, t.rcConfig.id)
102102
assert.strictEqual(version, ++receivedAckUpdates)
103103
assert.strictEqual(state, ACKNOWLEDGED)
104104
assert.notOk(error) // falsy check since error will be an empty string, but that's an implementation detail
105105

106106
endIfDone()
107-
}))
107+
})
108108

109-
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
109+
t.agent.on('debugger-diagnostics', ({ payload }) => {
110110
const expected = expectedPayloads.shift()
111111
assertObjectContains(payload, expected)
112112
assertUUID(payload.debugger.diagnostics.runtimeId)
113113
if (payload.debugger.diagnostics.status === 'INSTALLED') triggers.shift()()
114114
endIfDone()
115-
}))
115+
})
116116

117117
t.agent.addRemoteConfig(t.rcConfig)
118118

@@ -135,17 +135,17 @@ describe('Dynamic Instrumentation', function () {
135135
debugger: { diagnostics: { probeId, probeVersion: 0, status: 'INSTALLED' } }
136136
}]
137137

138-
t.agent.on('remote-config-ack-update', failOnException(done, (id, version, state, error) => {
138+
t.agent.on('remote-config-ack-update', (id, version, state, error) => {
139139
assert.strictEqual(id, t.rcConfig.id)
140140
assert.strictEqual(version, 1)
141141
assert.strictEqual(state, ACKNOWLEDGED)
142142
assert.notOk(error) // falsy check since error will be an empty string, but that's an implementation detail
143143

144144
receivedAckUpdate = true
145145
endIfDone()
146-
}))
146+
})
147147

148-
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
148+
t.agent.on('debugger-diagnostics', ({ payload }) => {
149149
const expected = expectedPayloads.shift()
150150
assertObjectContains(payload, expected)
151151
assertUUID(payload.debugger.diagnostics.runtimeId)
@@ -158,7 +158,7 @@ describe('Dynamic Instrumentation', function () {
158158
endIfDone()
159159
}, pollInterval * 2 * 1000) // wait twice as long as the RC poll interval
160160
}
161-
}))
161+
})
162162

163163
t.agent.addRemoteConfig(t.rcConfig)
164164

@@ -183,15 +183,15 @@ describe('Dynamic Instrumentation', function () {
183183
it(title, function (done) {
184184
let receivedAckUpdate = false
185185

186-
t.agent.on('remote-config-ack-update', failOnException(done, (id, version, state, error) => {
186+
t.agent.on('remote-config-ack-update', (id, version, state, error) => {
187187
assert.strictEqual(id, `logProbe_${config.id}`)
188188
assert.strictEqual(version, 1)
189189
assert.strictEqual(state, ERROR)
190190
assert.strictEqual(error.slice(0, 6), 'Error:')
191191

192192
receivedAckUpdate = true
193193
endIfDone()
194-
}))
194+
})
195195

196196
const probeId = config.id
197197
const expectedPayloads = [{
@@ -204,7 +204,7 @@ describe('Dynamic Instrumentation', function () {
204204
debugger: { diagnostics: customErrorDiagnosticsObj ?? { probeId, probeVersion: 0, status: 'ERROR' } }
205205
}]
206206

207-
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
207+
t.agent.on('debugger-diagnostics', ({ payload }) => {
208208
const expected = expectedPayloads.shift()
209209
assertObjectContains(payload, expected)
210210
const { diagnostics } = payload.debugger
@@ -218,7 +218,7 @@ describe('Dynamic Instrumentation', function () {
218218
}
219219

220220
endIfDone()
221-
}))
221+
})
222222

223223
t.agent.addRemoteConfig({
224224
product: 'LIVE_DEBUGGING',
@@ -237,7 +237,7 @@ describe('Dynamic Instrumentation', function () {
237237
it('should capture and send expected payload when a log line probe is triggered', function (done) {
238238
t.triggerBreakpoint()
239239

240-
t.agent.on('debugger-input', failOnException(done, ({ payload }) => {
240+
t.agent.on('debugger-input', ({ payload }) => {
241241
const expected = {
242242
ddsource: 'dd_debugger',
243243
hostname: os.hostname(),
@@ -284,7 +284,7 @@ describe('Dynamic Instrumentation', function () {
284284
assert.strictEqual(topFrame.columnNumber, 3)
285285

286286
done()
287-
}))
287+
})
288288

289289
t.agent.addRemoteConfig(t.rcConfig)
290290
})
@@ -307,31 +307,31 @@ describe('Dynamic Instrumentation', function () {
307307
if (payload.debugger.diagnostics.status === 'INSTALLED') triggers.shift()().catch(done)
308308
})
309309

310-
t.agent.on('debugger-input', failOnException(done, ({ payload }) => {
310+
t.agent.on('debugger-input', ({ payload }) => {
311311
assert.strictEqual(payload.message, expectedMessages.shift())
312312
if (expectedMessages.length === 0) done()
313-
}))
313+
})
314314

315315
t.agent.addRemoteConfig(t.rcConfig)
316316
})
317317

318318
it('should not trigger if probe is deleted', function (done) {
319-
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
319+
t.agent.on('debugger-diagnostics', ({ payload }) => {
320320
if (payload.debugger.diagnostics.status === 'INSTALLED') {
321-
t.agent.once('remote-confg-responded', failOnException(done, async () => {
321+
t.agent.once('remote-confg-responded', async () => {
322322
await t.axios.get('/foo')
323323
// We want to wait enough time to see if the client triggers on the breakpoint so that the test can fail
324324
// if it does, but not so long that the test times out.
325325
// TODO: Is there some signal we can use instead of a timer?
326326
setTimeout(done, pollInterval * 2 * 1000) // wait twice as long as the RC poll interval
327-
}))
327+
})
328328

329329
t.agent.removeRemoteConfig(t.rcConfig.id)
330330
}
331-
}))
331+
})
332332

333333
t.agent.on('debugger-input', () => {
334-
done(new Error('should not capture anything when the probe is deleted'))
334+
assert.fail('should not capture anything when the probe is deleted')
335335
})
336336

337337
t.agent.addRemoteConfig(t.rcConfig)
@@ -342,8 +342,7 @@ describe('Dynamic Instrumentation', function () {
342342
it('should remove the last breakpoint completely before trying to add a new one', function (done) {
343343
const rcConfig2 = t.generateRemoteConfig()
344344

345-
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
346-
const { status, probeId } = payload.debugger.diagnostics
345+
t.agent.on('debugger-diagnostics', ({ payload: { debugger: { diagnostics: { status, probeId } } } }) => {
347346
if (status !== 'INSTALLED') return
348347

349348
if (probeId === t.rcConfig.config.id) {
@@ -376,7 +375,7 @@ describe('Dynamic Instrumentation', function () {
376375
if (!finished) done(err)
377376
})
378377
}
379-
}))
378+
})
380379

381380
t.agent.addRemoteConfig(t.rcConfig)
382381
})

‎integration-tests/debugger/snapshot-pruning.spec.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
const { assert } = require('chai')
44
const { setup, getBreakpointInfo } = require('./utils')
5-
const { failOnException } = require('../helpers')
65

76
const { line } = getBreakpointInfo()
87

@@ -14,7 +13,7 @@ describe('Dynamic Instrumentation', function () {
1413
beforeEach(t.triggerBreakpoint)
1514

1615
it('should prune snapshot if payload is too large', function (done) {
17-
t.agent.on('debugger-input', failOnException(done, ({ payload }) => {
16+
t.agent.on('debugger-input', ({ payload }) => {
1817
assert.isBelow(Buffer.byteLength(JSON.stringify(payload)), 1024 * 1024) // 1MB
1918
assert.deepEqual(payload['debugger.snapshot'].captures, {
2019
lines: {
@@ -27,7 +26,7 @@ describe('Dynamic Instrumentation', function () {
2726
}
2827
})
2928
done()
30-
}))
29+
})
3130

3231
t.agent.addRemoteConfig(t.generateRemoteConfig({
3332
captureSnapshot: true,

‎integration-tests/debugger/snapshot.spec.js

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
const { assert } = require('chai')
44
const { setup } = require('./utils')
5-
const { failOnException } = require('../helpers')
65

76
describe('Dynamic Instrumentation', function () {
87
const t = setup()
@@ -12,7 +11,7 @@ describe('Dynamic Instrumentation', function () {
1211
beforeEach(t.triggerBreakpoint)
1312

1413
it('should capture a snapshot', function (done) {
15-
t.agent.on('debugger-input', failOnException(done, ({ payload: { 'debugger.snapshot': { captures } } }) => {
14+
t.agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => {
1615
assert.deepEqual(Object.keys(captures), ['lines'])
1716
assert.deepEqual(Object.keys(captures.lines), [String(t.breakpoint.line)])
1817

@@ -109,13 +108,13 @@ describe('Dynamic Instrumentation', function () {
109108
})
110109

111110
done()
112-
}))
111+
})
113112

114113
t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true }))
115114
})
116115

117116
it('should respect maxReferenceDepth', function (done) {
118-
t.agent.on('debugger-input', failOnException(done, ({ payload: { 'debugger.snapshot': { captures } } }) => {
117+
t.agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => {
119118
const { locals } = captures.lines[t.breakpoint.line]
120119
delete locals.request
121120
delete locals.fastify
@@ -145,13 +144,13 @@ describe('Dynamic Instrumentation', function () {
145144
})
146145

147146
done()
148-
}))
147+
})
149148

150149
t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxReferenceDepth: 0 } }))
151150
})
152151

153152
it('should respect maxLength', function (done) {
154-
t.agent.on('debugger-input', failOnException(done, ({ payload: { 'debugger.snapshot': { captures } } }) => {
153+
t.agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => {
155154
const { locals } = captures.lines[t.breakpoint.line]
156155

157156
assert.deepEqual(locals.lstr, {
@@ -162,13 +161,13 @@ describe('Dynamic Instrumentation', function () {
162161
})
163162

164163
done()
165-
}))
164+
})
166165

167166
t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxLength: 10 } }))
168167
})
169168

170169
it('should respect maxCollectionSize', function (done) {
171-
t.agent.on('debugger-input', failOnException(done, ({ payload: { 'debugger.snapshot': { captures } } }) => {
170+
t.agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => {
172171
const { locals } = captures.lines[t.breakpoint.line]
173172

174173
assert.deepEqual(locals.arr, {
@@ -183,7 +182,7 @@ describe('Dynamic Instrumentation', function () {
183182
})
184183

185184
done()
186-
}))
185+
})
187186

188187
t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxCollectionSize: 3 } }))
189188
})
@@ -206,7 +205,7 @@ describe('Dynamic Instrumentation', function () {
206205
}
207206
}
208207

209-
t.agent.on('debugger-input', failOnException(done, ({ payload: { 'debugger.snapshot': { captures } } }) => {
208+
t.agent.on('debugger-input', ({ payload: { 'debugger.snapshot': { captures } } }) => {
210209
const { locals } = captures.lines[t.breakpoint.line]
211210

212211
assert.deepEqual(Object.keys(locals), [
@@ -231,7 +230,7 @@ describe('Dynamic Instrumentation', function () {
231230
}
232231

233232
done()
234-
}))
233+
})
235234

236235
t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxFieldCount } }))
237236
})

‎integration-tests/helpers/fake-agent.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,14 @@ function buildExpressServer (agent) {
363363
})
364364
})
365365

366+
// Ensure that any failure inside of Express isn't swallowed and returned as a 500, but instead crashes the test
367+
app.use((err, req, res, next) => {
368+
if (!err) next()
369+
process.nextTick(() => {
370+
throw err
371+
})
372+
})
373+
366374
return app
367375
}
368376

‎integration-tests/helpers/index.js

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -356,26 +356,6 @@ function assertUUID (actual, msg = 'not a valid UUID') {
356356
assert.match(actual, /^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}$/, msg)
357357
}
358358

359-
function failOnException (done, fn) {
360-
if (fn[Symbol.toStringTag] === 'AsyncFunction') {
361-
return async (...args) => {
362-
try {
363-
await fn(...args)
364-
} catch (err) {
365-
done(err)
366-
}
367-
}
368-
} else {
369-
return (...args) => {
370-
try {
371-
fn(...args)
372-
} catch (err) {
373-
done(err)
374-
}
375-
}
376-
}
377-
}
378-
379359
module.exports = {
380360
FakeAgent,
381361
hookFile,
@@ -392,6 +372,5 @@ module.exports = {
392372
spawnPluginIntegrationTestProc,
393373
useEnv,
394374
useSandbox,
395-
sandboxCwd,
396-
failOnException
375+
sandboxCwd
397376
}

0 commit comments

Comments
 (0)