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

enable breadcrumbs and context-scoped calls for node #1927

Merged
merged 15 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 13 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

- (plugin-navigation-breadcrumbs) calling `pushState` or `replaceState` no longer triggers a new session when `autoTrackSessions` is enabled [#1820](https://github.com/bugsnag/bugsnag-js/pull/1820)
- (plugin-contextualize) reimplement without relying on the deprecated node Domain API. From Node 16+ unhandled promise rejections are also supported [#1924](https://github.com/bugsnag/bugsnag-js/pull/1924)
- (node) enable breadcrumbs and context-scoped calls [#1927](https://github.com/bugsnag/bugsnag-js/pull/1927)

## TBD

Expand Down
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ module.exports = {
], {
testEnvironment: 'node',
testMatch: [
'<rootDir>/packages/node/test/**/*.test.[jt]s',
'<rootDir>/packages/node/test/integration/**/*.test.[jt]s'
]
}),
Expand Down
21 changes: 8 additions & 13 deletions packages/node/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ const delivery = require('@bugsnag/delivery-node')
// extend the base config schema with some node-specific options
const schema = { ...require('@bugsnag/core/config').schema, ...require('./config') }

// remove enabledBreadcrumbTypes from the config schema
delete schema.enabledBreadcrumbTypes

const pluginApp = require('@bugsnag/plugin-app-duration')
const pluginSurroundingCode = require('@bugsnag/plugin-node-surrounding-code')
const pluginInProject = require('@bugsnag/plugin-node-in-project')
Expand Down Expand Up @@ -63,12 +60,6 @@ const Bugsnag = {

bugsnag._logger.debug('Loaded!')

bugsnag.leaveBreadcrumb = function () {
bugsnag._logger.warn('Breadcrumbs are not supported in Node.js yet')
}

bugsnag._config.enabledBreadcrumbTypes = []
Comment on lines -66 to -70
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the patches that were preventing breadcrumbs from being used with the node notifier


return bugsnag
},
start: (opts) => {
Expand All @@ -87,10 +78,14 @@ const Bugsnag = {
Object.keys(Client.prototype).forEach((m) => {
if (/^_/.test(m)) return
Bugsnag[m] = function () {
if (!Bugsnag._client) return console.error(`Bugsnag.${m}() was called before Bugsnag.start()`)
Bugsnag._client._depth += 1
const ret = Bugsnag._client[m].apply(Bugsnag._client, arguments)
Bugsnag._client._depth -= 1
// if we are in an async context, use the client from that context
const client = Bugsnag._client._clientContext.getStore() || Bugsnag._client

if (!client) return console.error(`Bugsnag.${m}() was called before Bugsnag.start()`)

client._depth += 1
const ret = client[m].apply(client, arguments)
client._depth -= 1
return ret
}
})
Expand Down
283 changes: 281 additions & 2 deletions packages/node/test/notifier.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Bugsnag from '..'
import Bugsnag from '../src/notifier'

describe('node notifier', () => {
beforeAll(() => {
Expand All @@ -7,7 +7,7 @@ describe('node notifier', () => {
})

beforeEach(() => {
// @ts-ignore:
// @ts-ignore
Bugsnag._client = null
})

Expand All @@ -21,4 +21,283 @@ describe('node notifier', () => {
expect(Bugsnag.isStarted()).toBe(true)
})
})

describe('addMetadata()', () => {
it('adds metadata to the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addMetadata('test', { meta: 'data' })
// @ts-ignore
expect(Bugsnag._client._metadata).toStrictEqual({ test: { meta: 'data' } })
})

describe('when in an async context', () => {
it('adds meta data to the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addMetadata('test', { meta: 'data' })
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._metadata).toStrictEqual({ test: { meta: 'data' } })
})

// @ts-ignore
expect(Bugsnag._client._metadata).toStrictEqual({})
})
})
})

describe('getMetadata()', () => {
it('retrieves metadata previously set on the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addMetadata('test', { meta: 'data' })
// @ts-ignore
expect(Bugsnag._client._metadata).toStrictEqual({ test: { meta: 'data' } })

expect(Bugsnag.getMetadata('test')).toStrictEqual({ meta: 'data' })
})

describe('when in an async context', () => {
it('retrieves metadata previously set on the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addMetadata('test', { meta: 'data' })
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._metadata).toStrictEqual({ test: { meta: 'data' } })

expect(Bugsnag.getMetadata('test')).toStrictEqual({ meta: 'data' })
})

// @ts-ignore
expect(Bugsnag._client._metadata).toStrictEqual({})
expect(Bugsnag.getMetadata('test')).toBeUndefined()
})
})
})

describe('clearMetadata()', () => {
it('clears metadata previously set on the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addMetadata('test', { meta: 'data' })
Bugsnag.clearMetadata('test')

// @ts-ignore
expect(Bugsnag._client._metadata).toStrictEqual({})
})

describe('when in an async context', () => {
it('clears metadata previously set on the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addMetadata('test', { meta: 'data' })
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addMetadata('test', { meta: 'data' })
Bugsnag.clearMetadata('test')
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._metadata).toStrictEqual({})
})

// @ts-ignore
expect(Bugsnag._client._metadata).toStrictEqual({ test: { meta: 'data' } })
})
})
})

describe('addFeatureFlag()', () => {
it('adds a feature flag to the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addFeatureFlag('test')
// @ts-ignore
expect(Bugsnag._client._features[0].name).toBe('test')
})

describe('when in an async context', () => {
it('adds a feature flag to the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addFeatureFlag('test')
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._features[0].name).toBe('test')
})

// @ts-ignore
expect(Bugsnag._client._features.length).toBe(0)
})
})
})

describe('addFeatureFlags()', () => {
it('adds feature flags to the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
// @ts-ignore
expect(Bugsnag._client._features).toStrictEqual([
{ name: 'test', variant: null },
{ name: 'other', variant: null }
])
})

describe('when in an async context', () => {
it('adds feature flags to the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._features).toStrictEqual([
{ name: 'test', variant: null },
{ name: 'other', variant: null }
])
})

// @ts-ignore
expect(Bugsnag._client._features).toStrictEqual([])
})
})
})

describe('clearFeatureFlag()', () => {
it('clears a feature flag set on the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
Bugsnag.clearFeatureFlag('test')
// @ts-ignore
expect(Bugsnag._client._features).toStrictEqual([
null,
{ name: 'other', variant: null }
])
})

describe('when in an async context', () => {
it('clears a feature flag previously set on the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
Bugsnag.clearFeatureFlag('test')
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._features).toStrictEqual([
null,
{ name: 'other', variant: null }
])
})

// @ts-ignore
expect(Bugsnag._client._features).toStrictEqual([
{ name: 'test', variant: null },
{ name: 'other', variant: null }
])
})
})
})

describe('clearFeatureFlags()', () => {
it('clears feature flags set on the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
Bugsnag.clearFeatureFlags()
// @ts-ignore
expect(Bugsnag._client._features).toStrictEqual([])
})

describe('when in an async context', () => {
it('clears feature flags previously set on the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.addFeatureFlags([{ name: 'test' }, { name: 'other' }])
Bugsnag.clearFeatureFlags()
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._features).toStrictEqual([])
})

// @ts-ignore
expect(Bugsnag._client._features).toStrictEqual([
{ name: 'test', variant: null },
{ name: 'other', variant: null }
])
})
})
})

describe('setContext() and getContext()', () => {
it('sets the context on the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
expect(Bugsnag.getContext()).toBeUndefined()
Bugsnag.setContext('my context')
expect(Bugsnag.getContext()).toBe('my context')
})

describe('when in an async context', () => {
it('sets the context on the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.setContext('my context')
expect(Bugsnag.getContext()).toBe('my context')
})

expect(Bugsnag.getContext()).toBeUndefined()
})
})
})

describe('setUser() and getUser()', () => {
it('sets the context on the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
expect(Bugsnag.getUser()).toStrictEqual({})
Bugsnag.setUser('my user id')
expect(Bugsnag.getUser()).toStrictEqual({ id: 'my user id', email: undefined, name: undefined })
})

describe('when in an async context', () => {
it('sets the context on the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.setUser('my user id')
expect(Bugsnag.getUser()).toStrictEqual({ id: 'my user id', email: undefined, name: undefined })
})

expect(Bugsnag.getUser()).toStrictEqual({})
})
})
})

describe('leaveBreadcrumb()', () => {
it('adds a breadcrumb to the client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
Bugsnag.leaveBreadcrumb('test')
// @ts-ignore
expect(Bugsnag._client._breadcrumbs[0].message).toBe('test')
})

describe('when in an async context', () => {
it('adds a breadcrumb to the cloned client not not the base client', () => {
Bugsnag.start('abcd12abcd12abcd12abcd12abcd12abcd')
const contextualize = Bugsnag.getPlugin('contextualize')

contextualize(() => {
Bugsnag.leaveBreadcrumb('test')
// @ts-ignore
expect(Bugsnag._client._clientContext.getStore()._breadcrumbs[0].message).toBe('test')
})

// @ts-ignore
expect(Bugsnag._client._breadcrumbs.length).toBe(0)
})
})
})
})
6 changes: 5 additions & 1 deletion packages/plugin-express/src/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ module.exports = {
next(err)
}

return { requestHandler, errorHandler }
const runInContext = (req, res, next) => {
client._clientContext.run(req.bugsnag, next)
}
Comment on lines +58 to +60
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new express middleware that can be used to restore async local storage context if it is lost. See E2E tes showing usage


return { requestHandler, errorHandler, runInContext }
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/plugin-express/types/bugsnag-express.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export default bugsnagPluginExpress
interface BugsnagPluginExpressResult {
errorHandler: express.ErrorRequestHandler
requestHandler: express.RequestHandler
runInContext: express.RequestHandler
}

// add a new call signature for the getPlugin() method that types the plugin result
Expand Down
Loading
Loading