Skip to content

Commit d6d080c

Browse files
committed
Use errorMonitor symbol instead of listening to the error event
This is important to guarantee user code behaves as it did without the instrumentation. So user crashes should happen as before.
1 parent 3a2ed10 commit d6d080c

File tree

13 files changed

+43
-28
lines changed

13 files changed

+43
-28
lines changed

packages/datadog-instrumentations/src/child_process.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const { errorMonitor } = require('node:events')
34
const util = require('util')
45

56
const {
@@ -227,7 +228,7 @@ function wrapChildProcessAsyncMethod (ChildProcess, shell = false) {
227228
if (childProcess) {
228229
let errorExecuted = false
229230

230-
childProcess.on('error', (e) => {
231+
childProcess.on(errorMonitor, (e) => {
231232
errorExecuted = true
232233
childProcessChannel.error.publish(e)
233234
})

packages/datadog-instrumentations/src/couchbase.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const { errorMonitor } = require('events')
34
const {
45
channel,
56
addHook,
@@ -186,7 +187,7 @@ addHook({ name: 'couchbase', file: 'lib/bucket.js', versions: ['^2.6.12'] }, Buc
186187
finishCh.publish(undefined)
187188
}))
188189

189-
emitter.once('error', asyncResource.bind((error) => {
190+
emitter.once(errorMonitor, asyncResource.bind((error) => {
190191
errorCh.publish(error)
191192
finishCh.publish(undefined)
192193
}))

packages/datadog-instrumentations/src/fs.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const { errorMonitor } = require('events')
34
const { channel, addHook } = require('./helpers/instrument')
45
const shimmer = require('../../datadog-shimmer')
56

@@ -199,16 +200,16 @@ function wrapCreateStream (original) {
199200
}
200201
const onFinish = () => {
201202
finishChannel.runStores(ctx, () => {})
202-
stream.off('close', onFinish)
203-
stream.off('end', onFinish)
204-
stream.off('finish', onFinish)
205-
stream.off('error', onError)
203+
stream.removeListener('close', onFinish)
204+
stream.removeListener('end', onFinish)
205+
stream.removeListener('finish', onFinish)
206+
stream.removeListener(errorMonitor, onError)
206207
}
207208

208209
stream.once('close', onFinish)
209210
stream.once('end', onFinish)
210211
stream.once('finish', onFinish)
211-
stream.once('error', onError)
212+
stream.once(errorMonitor, onError)
212213

213214
return stream
214215
} catch (error) {

packages/datadog-instrumentations/src/http/client.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/* eslint-disable no-fallthrough */
44

55
const url = require('url')
6+
const { errorMonitor } = require('node:events')
67
const { channel, addHook } = require('../helpers/instrument')
78
const shimmer = require('../../../datadog-shimmer')
89

@@ -88,7 +89,7 @@ function patch (http, methodName) {
8889
const res = arg
8990
ctx.res = res
9091
res.on('end', finish)
91-
res.on('error', finish)
92+
res.on(errorMonitor, finish)
9293
break
9394
}
9495
case 'connect':

packages/datadog-instrumentations/src/mysql2.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
'use strict'
22

3+
const { errorMonitor } = require('node:events')
4+
35
const {
46
channel,
57
addHook,
@@ -138,7 +140,7 @@ function wrapConnection (Connection, version) {
138140
onResult.apply(this, arguments)
139141
}, 'bound-anonymous-fn', this))
140142
} else {
141-
this.on('error', asyncResource.bind(error => errorCh.publish(error)))
143+
this.on(errorMonitor, asyncResource.bind(error => errorCh.publish(error)))
142144
this.on('end', asyncResource.bind(() => finishCh.publish(undefined)))
143145
}
144146

packages/datadog-instrumentations/src/net.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
'use strict'
22

3+
const { errorMonitor } = require('events')
4+
35
const { channel, addHook } = require('./helpers/instrument')
46
const shimmer = require('../../datadog-shimmer')
57

@@ -99,7 +101,7 @@ function getOptions (args) {
99101
}
100102

101103
function setupListeners (socket, protocol, ctx, finishCh, errorCh) {
102-
const events = ['connect', 'error', 'close', 'timeout']
104+
const events = ['connect', errorMonitor, 'close', 'timeout']
103105

104106
const wrapListener = function (error) {
105107
if (error) {

packages/datadog-instrumentations/src/pg.js

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,21 +129,12 @@ function wrapQuery (query) {
129129
}
130130
} else if (newQuery.once) {
131131
newQuery
132-
.once('error', finish)
132+
.once(errorMonitor, finish)
133133
.once('end', (res) => finish(null, res))
134134
} else {
135135
newQuery.then((res) => finish(null, res), finish)
136136
}
137137

138-
if (stream) {
139-
newQuery.on('end', () => {
140-
finish(null, [])
141-
})
142-
newQuery.on(errorMonitor, (err) => {
143-
finish(err)
144-
})
145-
}
146-
147138
try {
148139
return retval
149140
} catch (err) {

packages/datadog-instrumentations/test/child_process.spec.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ describe('child process', () => {
102102
it('should execute error callback', (done) => {
103103
const childEmitter = childProcess[methodName]('invalid_command_test')
104104

105+
expect(childEmitter.listenerCount('error')).to.equal(methodName.includes('spawn') ? 0 : 1)
106+
107+
childEmitter.once('error', () => {})
108+
105109
childEmitter.once('close', () => {
106110
expect(start).to.have.been.calledOnceWith({
107111
command: 'invalid_command_test',

packages/datadog-instrumentations/test/mysql2.spec.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const { channel } = require('../src/helpers/instrument')
44
const agent = require('../../dd-trace/test/plugins/agent')
55
const { assert } = require('chai')
66
const semver = require('semver')
7+
const { once } = require('events')
78

89
describe('mysql2 instrumentation', () => {
910
withVersions('mysql2', 'mysql2', version => {
@@ -435,16 +436,18 @@ describe('mysql2 instrumentation', () => {
435436
query.on('end', () => done())
436437
})
437438

438-
it('should work without abortController.abort()', (done) => {
439+
it('should work without abortController.abort()', async () => {
439440
startCh.subscribe(noop)
440441
const query = pool.query(sql)
441442

442-
query.on('error', err => done(err))
443-
query.on('end', () => {
444-
sinon.assert.called(apmQueryStart)
443+
let error
444+
query.on('error', err => { error = err })
445+
expect(query.listenerCount('error')).to.equal(1)
445446

446-
done()
447-
})
447+
await once(query, 'end')
448+
expect(query.listenerCount('error')).to.equal(1)
449+
sinon.assert.called(apmQueryStart)
450+
expect(error).to.be.undefined
448451
})
449452

450453
it('should work without subscriptions', (done) => {

packages/datadog-plugin-fs/test/index.spec.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,10 @@ describe('Plugin', () => {
675675
'file.flag': 'r'
676676
}
677677
})
678-
fs.createReadStream(__filename).on('error', done).resume()
678+
const stream = fs.createReadStream(__filename)
679+
expect(stream.listenerCount('error')).to.equal(0)
680+
681+
stream.on('error', done).resume()
679682
})
680683

681684
it('should be instrumented when closed', (done) => {

0 commit comments

Comments
 (0)