Skip to content

Commit

Permalink
Merge pull request #658 from bugsnag/v7-metadata-refactor
Browse files Browse the repository at this point in the history
V7: Metadata refactor
  • Loading branch information
bengourley committed Dec 3, 2019
2 parents b12df27 + c1efd3b commit 7e70758
Show file tree
Hide file tree
Showing 53 changed files with 334 additions and 268 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Update signature of `notify(err, opts?, cb?)` -> `notify(err, onError?, cb?)` for a canonical way to update events [#655](https://github.com/bugsnag/bugsnag-js/pull/655)
- Simplify client configuration, and store resulting config privately [#656](https://github.com/bugsnag/bugsnag-js/pull/656)
- User is now stored privately on `client` and `event` and updated via get/set methods [#657](https://github.com/bugsnag/bugsnag-js/pull/657)
- Rename `metaData` -> `metadata` and add consistent `add/get/clearMetadata()` methods to `Client`/`Event` for manipulating metadata explicitly, rather than mutating a property [#658](https://github.com/bugsnag/bugsnag-js/pull/658)

## 6.4.3 (2019-10-21)

Expand Down
4 changes: 2 additions & 2 deletions examples/browser-cdn/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ var bugsnagClient = bugsnag({

// add any custom attributes relevant to your app. Note that metadata can be
// added here, in a specific notify() or in an onError.
metaData: {
metadata: {
company: {
name: "Xavier's School for Gifted Youngsters"
}
Expand All @@ -82,7 +82,7 @@ function sendHandled () {
// Note that metadata can be declared globally, in the notification (as below) or in an onError.
// The below metadata will be supplemented (not replaced) by the metadata
// in the onError method. See our docs if you prefer to overwrite/remove metadata.
event.addMetaData('details', {
event.addMetadata('details', {
info: 'Any important details specific to the context of this particular error/function.'
})
})
Expand Down
2 changes: 1 addition & 1 deletion examples/nuxtjs/modules/bugsnag/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ export default function (options) {
this.nuxt.hook('render:setupMiddleware', app => app.use(bugsnagClient.getPlugin('express').requestHandler))
this.nuxt.hook('render:errorMiddleware', app => app.use(bugsnagClient.getPlugin('express').errorHandler))
this.nuxt.hook('generate:routeFailed', ({ route, errors }) => {
errors.forEach(({ error }) => bugsnagClient.notify(error, event => { event.addMetaData({ route }) }))
errors.forEach(({ error }) => bugsnagClient.notify(error, event => { event.addMetadata({ route }) }))
})
}
4 changes: 2 additions & 2 deletions examples/plain-node/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ var bugsnagClient = bugsnag({
// only get reports from the environments you care about
releaseStage: 'production',
enabledReleaseStages: [ 'staging', 'production' ],
// you can set global metaData when you initialise Bugsnag
metaData: {}
// you can set global metadata when you initialise Bugsnag
metadata: {}
})

console.log(`
Expand Down
4 changes: 2 additions & 2 deletions examples/typescript/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const bugsnagClient = bugsnag({
// only get reports from the environments you care about
releaseStage: 'production',
enabledReleaseStages: [ 'staging', 'production' ],
// you can set global metaData when you initialise Bugsnag
metaData: {}
// you can set global metadata when you initialise Bugsnag
metadata: {}
})

console.log(`
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/types/bugsnag.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ declare module "@bugsnag/core" {
releaseStage?: string;
maxBreadcrumbs?: number;
user?: object | null;
metaData?: object | null;
metadata?: { [key: string]: any };
logger?: BugsnagCore.Logger | null;
filters?: Array<string | RegExp>;
// catch-all for any missing options
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/types/test/fixtures/all-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ bugsnag({
maxBreadcrumbs: 20,
enabledBreadcrumbTypes: ['manual','log','request'],
user: null,
metaData: null,
metaData: {},
logger: undefined,
filters: ["foo",/bar/],
collectUserIp: true,
Expand Down
20 changes: 17 additions & 3 deletions packages/core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const { map, includes } = require('./lib/es-utils')
const inferReleaseStage = require('./lib/infer-release-stage')
const isError = require('./lib/iserror')
const runCallbacks = require('./lib/callback-runner')
const metadataDelegate = require('./lib/metadata-delegate')

const LOG_USAGE_ERR_PREFIX = 'Usage error.'
const REPORT_USAGE_ERR_PREFIX = 'Bugsnag usage error.'
Expand All @@ -29,13 +30,14 @@ class BugsnagClient {

this._session = null

this._metadata = {}

this.breadcrumbs = []

// setable props
this.app = {}
this.context = undefined
this.device = undefined
this.metaData = undefined
this.request = undefined
this._user = {}

Expand All @@ -54,6 +56,18 @@ class BugsnagClient {
}
}

addMetadata (section, ...args) {
return metadataDelegate.add(this._metadata, section, ...args)
}

getMetadata (section, key) {
return metadataDelegate.get(this._metadata, section, key)
}

clearMetadata (section, key) {
return metadataDelegate.clear(this._metadata, section, key)
}

_extractConfiguration (partialSchema = this._schema) {
const conf = config.mergeDefaults(this._opts, partialSchema)
const validity = config.validate(conf, partialSchema)
Expand All @@ -64,7 +78,7 @@ class BugsnagClient {
if (typeof conf.onError === 'function') conf.onError = [conf.onError]
if (conf.appVersion) this.app.version = conf.appVersion
if (conf.appType) this.app.type = conf.appType
if (conf.metaData) this.metaData = conf.metaData
if (conf.metadata) this._metadata = conf.metadata
if (conf.user) this._user = conf.user
if (conf.logger) this.logger(conf.logger)

Expand Down Expand Up @@ -157,8 +171,8 @@ class BugsnagClient {
event.context = event.context || this.context || undefined
event.device = { ...event.device, ...this.device }
event.request = { ...event.request, ...this.request }
event._metadata = { ...event._metadata, ...this._metadata }
event._user = { ...event._user, ...this._user }
event.metaData = { ...event.metaData, ...this.metaData }
event.breadcrumbs = this.breadcrumbs.slice(0)

if (this._session) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ module.exports.schema = {
message: '(object) user should be an object',
validate: (value) => typeof value === 'object'
},
metaData: {
metadata: {
defaultValue: () => null,
message: 'should be an object',
validate: (value) => typeof value === 'object'
Expand Down
51 changes: 10 additions & 41 deletions packages/core/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const StackGenerator = require('stack-generator')
const hasStack = require('./lib/has-stack')
const { reduce, filter } = require('./lib/es-utils')
const jsRuntime = require('./lib/js-runtime')
const metadataDelegate = require('./lib/metadata-delegate')

class BugsnagEvent {
constructor (errorClass, errorMessage, stacktrace = [], handledState = defaultHandledState(), originalError) {
Expand All @@ -21,7 +22,7 @@ class BugsnagEvent {
this.errorClass = stringOrFallback(errorClass, '[no error class]')
this.errorMessage = stringOrFallback(errorMessage, '[no error message]')
this.groupingHash = undefined
this.metaData = {}
this._metadata = {}
this.request = undefined
this.severity = this._handledState.severity
this.stacktrace = reduce(stacktrace, (accum, frame) => {
Expand All @@ -45,48 +46,16 @@ class BugsnagEvent {
/* this.attemptImmediateDelivery, default: true */
}

updateMetaData (section, ...args) {
if (!section) return this
let updates

// updateMetaData("section", null) -> removes section
if (args[0] === null) return this.removeMetaData(section)

// updateMetaData("section", "property", null) -> removes property from section
if (args[1] === null) return this.removeMetaData(section, args[0], args[1])

// normalise the two supported input types into object form
if (typeof args[0] === 'object') updates = args[0]
if (typeof args[0] === 'string') updates = { [args[0]]: args[1] }

// exit if we don't have an updates object at this point
if (!updates) return this

// ensure a section with this name exists
if (!this.metaData[section]) this.metaData[section] = {}

// merge the updates with the existing section
this.metaData[section] = { ...this.metaData[section], ...updates }

return this
addMetadata (section, ...args) {
return metadataDelegate.add(this._metadata, section, ...args)
}

removeMetaData (section, property) {
if (typeof section !== 'string') return this

// remove an entire section
if (!property) {
delete this.metaData[section]
return this
}

// remove a single property from a section
if (this.metaData[section]) {
delete this.metaData[section][property]
return this
}
getMetadata (section, key) {
return metadataDelegate.get(this._metadata, section, key)
}

return this
clearMetadata (section, key) {
return metadataDelegate.clear(this._metadata, section, key)
}

getUser () {
Expand Down Expand Up @@ -115,8 +84,8 @@ class BugsnagEvent {
device: this.device,
breadcrumbs: this.breadcrumbs,
context: this.context,
metaData: this._metadata,
user: this._user,
metaData: this.metaData,
groupingHash: this.groupingHash,
request: this.request,
session: this.session
Expand Down
2 changes: 1 addition & 1 deletion packages/core/lib/clone-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = (client) => {
// changes to these properties should not be reflected in the original client,
// so ensure they are are (shallow) cloned
clone.breadcrumbs = client.breadcrumbs.slice()
clone.metaData = { ...client.metaData }
clone._metadata = { ...client._metadata }
clone.request = { ...client.request }
clone._user = { ...client._user }

Expand Down
2 changes: 1 addition & 1 deletion packages/core/lib/event-from-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ module.exports = (maybeError, handledState) => {
handledState,
maybeError
)
if (maybeError !== actualError) event.updateMetaData('error', 'non-error value', String(maybeError))
if (maybeError !== actualError) event.addMetadata('error', 'non-error value', String(maybeError))
return event
}
5 changes: 2 additions & 3 deletions packages/core/lib/json-payload.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ const SESSION_FILTER_PATHS = [
module.exports.event = (event, filterKeys) => {
let payload = jsonStringify(event, null, null, { filterPaths: EVENT_FILTER_PATHS, filterKeys })
if (payload.length > 10e5) {
delete event.events[0].metaData
event.events[0].metaData = {
event.events[0]._metadata = {
notifier:
`WARNING!
Serialized payload was ${payload.length / 10e5}MB (limit = 1MB)
metaData was removed`
metadata was removed`
}
payload = jsonStringify(event, null, null, { filterPaths: EVENT_FILTER_PATHS, filterKeys })
if (payload.length > 10e5) throw new Error('payload exceeded 1MB limit')
Expand Down
48 changes: 48 additions & 0 deletions packages/core/lib/metadata-delegate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
const add = (state, section, ...args) => {
if (!section) return
let updates

// addMetadata("section", null) -> clears section
if (args[0] === null) return clear(state, section)

// normalise the two supported input types into object form
if (typeof args[0] === 'object') updates = args[0]
if (typeof args[0] === 'string') updates = { [args[0]]: args[1] }

// exit if we don't have an updates object at this point
if (!updates) return

// ensure a section with this name exists
if (!state[section]) state[section] = {}

// merge the updates with the existing section
state[section] = { ...state[section], ...updates }
}

const get = (state, section, key) => {
if (typeof section !== 'string') return undefined
if (!key) {
return state[section]
}
if (state[section]) {
return state[section][key]
}
return undefined
}

const clear = (state, section, key) => {
if (typeof section !== 'string') return

// clear an entire section
if (!key) {
delete state[section]
return
}

// clear a single value from a section
if (state[section]) {
delete state[section][key]
}
}

module.exports = { add, get, clear }
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('createEventFromErr(maybeErr)', () => {
it('tolerates null', () => {
const event = createEventFromErr(null)
expect(event.errorMessage).toBe('Handled a non-error. See "error" tab for more detail.')
expect(event.metaData.error['non-error value']).toBe('null')
expect(event._metadata.error['non-error value']).toBe('null')
})

it('accepts acustom handledState', () => {
Expand Down
33 changes: 29 additions & 4 deletions packages/core/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,13 @@ describe('@bugsnag/core/client', () => {
expect(payloads[0].events[0].breadcrumbs.length).toBe(0)
})

it('doesn’t modify global client.metaData when using updateMetaData() method', () => {
it('doesn’t modify global client.metadata when using addMetadata() method', () => {
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.metaData = { foo: [1, 2, 3] }
client.addMetadata('foo', 'bar', [1, 2, 3])
client.notify(new Error('changes afoot'), (event) => {
event.updateMetaData('foo', '3', 1)
event.addMetadata('foo', '3', 1)
})
expect(client.metaData.foo['3']).toBe(undefined)
expect(client._metadata.foo['3']).toBe(undefined)
})

it('should call the callback (success)', done => {
Expand Down Expand Up @@ -476,6 +476,31 @@ describe('@bugsnag/core/client', () => {
})
})

describe('add/get/clearMetadata()', () => {
it('modifies and retrieves metadata', () => {
const client = new Client({ apiKey: 'API_KEY' })
client.addMetadata('a', 'b', 'c')
expect(client.getMetadata('a')).toEqual({ b: 'c' })
expect(client.getMetadata('a', 'b')).toBe('c')
client.clearMetadata('a', 'b')
expect(client.getMetadata('a', 'b')).toBe(undefined)
client.clearMetadata('a')
expect(client.getMetadata('a')).toBe(undefined)
})

it('can be set in config', () => {
const client = new Client({
apiKey: 'API_KEY',
metadata: {
'system metrics': {
ms_since_last_jolt: 10032
}
}
})
expect(client.getMetadata('system metrics', 'ms_since_last_jolt')).toBe(10032)
})
})

describe('getUser() / setUser()', () => {
it('sets and retrieves user properties', () => {
const c = new Client({ apiKey: 'aaaa' })
Expand Down
Loading

0 comments on commit 7e70758

Please sign in to comment.