Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DI] Handle async errors in mocha tests #4991

Merged
merged 3 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 37 additions & 48 deletions integration-tests/debugger/basic.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const os = require('os')

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

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

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

receivedAckUpdate = true
endIfDone()
})
}))

t.agent.on('debugger-diagnostics', ({ payload }) => {
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
const expected = expectedPayloads.shift()
assertObjectContains(payload, expected)
assertUUID(payload.debugger.diagnostics.runtimeId)
Expand All @@ -60,7 +60,7 @@ describe('Dynamic Instrumentation', function () {
} else {
endIfDone()
}
})
}))

t.agent.addRemoteConfig(t.rcConfig)

Expand Down Expand Up @@ -97,22 +97,22 @@ describe('Dynamic Instrumentation', function () {
() => {}
]

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

endIfDone()
})
}))

t.agent.on('debugger-diagnostics', ({ payload }) => {
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
const expected = expectedPayloads.shift()
assertObjectContains(payload, expected)
assertUUID(payload.debugger.diagnostics.runtimeId)
if (payload.debugger.diagnostics.status === 'INSTALLED') triggers.shift()()
endIfDone()
})
}))

t.agent.addRemoteConfig(t.rcConfig)

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

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

receivedAckUpdate = true
endIfDone()
})
}))

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

t.agent.addRemoteConfig(t.rcConfig)

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

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

receivedAckUpdate = true
endIfDone()
})
}))

const probeId = config.id
const expectedPayloads = [{
Expand All @@ -201,10 +201,10 @@ describe('Dynamic Instrumentation', function () {
}, {
ddsource: 'dd_debugger',
service: 'node',
debugger: { diagnostics: customErrorDiagnosticsObj ?? { probeId, version: 0, status: 'ERROR' } }
debugger: { diagnostics: customErrorDiagnosticsObj ?? { probeId, probeVersion: 0, status: 'ERROR' } }
}]

t.agent.on('debugger-diagnostics', ({ payload }) => {
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
const expected = expectedPayloads.shift()
assertObjectContains(payload, expected)
const { diagnostics } = payload.debugger
Expand All @@ -218,7 +218,7 @@ describe('Dynamic Instrumentation', function () {
}

endIfDone()
})
}))

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

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

done()
})
}))

t.agent.addRemoteConfig(t.rcConfig)
})
Expand All @@ -307,43 +307,31 @@ describe('Dynamic Instrumentation', function () {
if (payload.debugger.diagnostics.status === 'INSTALLED') triggers.shift()().catch(done)
})

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

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

it('should not trigger if probe is deleted', function (done) {
t.agent.on('debugger-diagnostics', async ({ payload }) => {
try {
if (payload.debugger.diagnostics.status === 'INSTALLED') {
t.agent.once('remote-confg-responded', async () => {
try {
await t.axios.get('/foo')
// We want to wait enough time to see if the client triggers on the breakpoint so that the test can fail
// if it does, but not so long that the test times out.
// TODO: Is there some signal we can use instead of a timer?
setTimeout(done, pollInterval * 2 * 1000) // wait twice as long as the RC poll interval
} catch (err) {
// Nessecary hack: Any errors thrown inside of an async function is invisible to Mocha unless the outer
// `it` callback is also `async` (which we can't do in this case since we rely on the `done` callback).
done(err)
}
})
t.agent.on('debugger-diagnostics', failOnException(done, ({ payload }) => {
if (payload.debugger.diagnostics.status === 'INSTALLED') {
t.agent.once('remote-confg-responded', failOnException(done, async () => {
await t.axios.get('/foo')
// We want to wait enough time to see if the client triggers on the breakpoint so that the test can fail
// if it does, but not so long that the test times out.
// TODO: Is there some signal we can use instead of a timer?
setTimeout(done, pollInterval * 2 * 1000) // wait twice as long as the RC poll interval
}))

t.agent.removeRemoteConfig(t.rcConfig.id)
}
} catch (err) {
// Nessecary hack: Any errors thrown inside of an async function is invisible to Mocha unless the outer `it`
// callback is also `async` (which we can't do in this case since we rely on the `done` callback).
done(err)
t.agent.removeRemoteConfig(t.rcConfig.id)
}
})
}))

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

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

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

if (probeId === t.rcConfig.config.id) {
Expand Down Expand Up @@ -387,7 +376,7 @@ describe('Dynamic Instrumentation', function () {
if (!finished) done(err)
})
}
})
}))

t.agent.addRemoteConfig(t.rcConfig)
})
Expand Down
5 changes: 3 additions & 2 deletions integration-tests/debugger/snapshot-pruning.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const { assert } = require('chai')
const { setup, getBreakpointInfo } = require('./utils')
const { failOnException } = require('../helpers')

const { line } = getBreakpointInfo()

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

it('should prune snapshot if payload is too large', function (done) {
t.agent.on('debugger-input', ({ payload }) => {
t.agent.on('debugger-input', failOnException(done, ({ payload }) => {
assert.isBelow(Buffer.byteLength(JSON.stringify(payload)), 1024 * 1024) // 1MB
assert.deepEqual(payload['debugger.snapshot'].captures, {
lines: {
Expand All @@ -26,7 +27,7 @@ describe('Dynamic Instrumentation', function () {
}
})
done()
})
}))

t.agent.addRemoteConfig(t.generateRemoteConfig({
captureSnapshot: true,
Expand Down
21 changes: 11 additions & 10 deletions integration-tests/debugger/snapshot.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const { assert } = require('chai')
const { setup } = require('./utils')
const { failOnException } = require('../helpers')

describe('Dynamic Instrumentation', function () {
const t = setup()
Expand All @@ -11,7 +12,7 @@ describe('Dynamic Instrumentation', function () {
beforeEach(t.triggerBreakpoint)

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

Expand Down Expand Up @@ -108,13 +109,13 @@ describe('Dynamic Instrumentation', function () {
})

done()
})
}))

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

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

done()
})
}))

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

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

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

done()
})
}))

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

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

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

done()
})
}))

t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxCollectionSize: 3 } }))
})
Expand All @@ -205,7 +206,7 @@ describe('Dynamic Instrumentation', function () {
}
}

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

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

done()
})
}))

t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true, capture: { maxFieldCount } }))
})
Expand Down
23 changes: 22 additions & 1 deletion integration-tests/helpers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,26 @@ function assertUUID (actual, msg = 'not a valid UUID') {
assert.match(actual, /^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}$/, msg)
}

function failOnException (done, fn) {
if (fn[Symbol.toStringTag] === 'AsyncFunction') {
return async (...args) => {
try {
await fn(...args)
} catch (err) {
done(err)
}
}
} else {
return (...args) => {
try {
fn(...args)
} catch (err) {
done(err)
}
}
}
}

module.exports = {
FakeAgent,
hookFile,
Expand All @@ -372,5 +392,6 @@ module.exports = {
spawnPluginIntegrationTestProc,
useEnv,
useSandbox,
sandboxCwd
sandboxCwd,
failOnException
}
Loading