Skip to content

Commit

Permalink
Disable crashing pugins (#4440)
Browse files Browse the repository at this point in the history
* Disable crashing pugins

When one of the plugins subscriptions fails, disable the whole plugin.

* pr feedback

* fix logging

* lint
  • Loading branch information
bengl authored and juan-fernandez committed Aug 5, 2024
1 parent 1b9ee54 commit 679e7e2
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 1 deletion.
13 changes: 12 additions & 1 deletion packages/dd-trace/src/plugins/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// TODO: move anything related to tracing to TracingPlugin instead

const dc = require('dc-polyfill')
const logger = require('../log')
const { storage } = require('../../../datadog-core')

class Subscription {
Expand Down Expand Up @@ -72,7 +73,17 @@ module.exports = class Plugin {
}

addSub (channelName, handler) {
this._subscriptions.push(new Subscription(channelName, handler))
const plugin = this
const wrappedHandler = function () {
try {
return handler.apply(this, arguments)
} catch (e) {
logger.error('Error in plugin handler:', e)
logger.info('Disabling plugin:', plugin.id)
plugin.configure(false)
}
}
this._subscriptions.push(new Subscription(channelName, wrappedHandler))
}

addBind (channelName, transform) {
Expand Down
69 changes: 69 additions & 0 deletions packages/dd-trace/test/plugins/plugin.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict'

require('../setup/tap')

const Plugin = require('../../src/plugins/plugin')
const plugins = require('../../src/plugins')
const { channel } = require('dc-polyfill')

describe('Plugin', () => {
class BadPlugin extends Plugin {
static get id () { return 'badPlugin' }

constructor () {
super()
this.addSub('apm:badPlugin:start', this.start)
}

start () {
throw new Error('this is one bad plugin')
}
}

class GoodPlugin extends Plugin {
static get id () { return 'goodPlugin' }

constructor () {
super()
this.addSub('apm:goodPlugin:start', this.start)
}

start () {
//
}
}

const testPlugins = { badPlugin: BadPlugin, goodPlugin: GoodPlugin }
const loadChannel = channel('dd-trace:instrumentation:load')

before(() => {
for (const [name, cls] of Object.entries(testPlugins)) {
plugins[name] = cls
loadChannel.publish({ name })
}
})

after(() => { Object.keys(testPlugins).forEach(name => delete plugins[name]) })

it('should disable upon error', () => {
const plugin = new BadPlugin()
plugin.configure({ enabled: true })

expect(plugin._enabled).to.be.true

channel('apm:badPlugin:start').publish({ foo: 'bar' })

expect(plugin._enabled).to.be.false
})

it('should not disable with no error', () => {
const plugin = new GoodPlugin()
plugin.configure({ enabled: true })

expect(plugin._enabled).to.be.true

channel('apm:goodPlugin:start').publish({ foo: 'bar' })

expect(plugin._enabled).to.be.true
})
})

0 comments on commit 679e7e2

Please sign in to comment.