Skip to content

Commit

Permalink
Merge pull request #650 from bugsnag/v7-breadcrumb-refactor
Browse files Browse the repository at this point in the history
V7: Breadcrumb refactor
  • Loading branch information
bengourley committed Nov 19, 2019
2 parents a8af95f + b245c79 commit 8917cbc
Show file tree
Hide file tree
Showing 40 changed files with 158 additions and 240 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Rename `autoCaptureSessions` -> `autoTrackSessions` and simplify validation logic [#647](https://github.com/bugsnag/bugsnag-js/pull/647)
- Rename `report` to `event` [#646](https://github.com/bugsnag/bugsnag-js/pull/646)
- Rename `notifyReleaseStages` -> `enabledReleaseStages` [#649](https://github.com/bugsnag/bugsnag-js/pull/649)
- Remove individual breadcrumb flags in favour of `enabledBreadcrumbTypes`, rename `breadcrumb.{ name -> message, metaData -> metadata }`, and update `leaveBreadcrumb()` type signature to be more explicit [#650](https://github.com/bugsnag/bugsnag-js/pull/649)

## 6.4.3 (2019-10-21)

Expand Down
6 changes: 1 addition & 5 deletions packages/browser/types/bugsnag.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ declare module "@bugsnag/core" {
interface Config {
apiKey: string;
beforeSend?: BugsnagCore.BeforeSend | BugsnagCore.BeforeSend[];
autoBreadcrumbs?: boolean;
autoDetectErrors?: boolean;
autoDetectUnhandledRejections?: boolean;
enabledBreadcrumbTypes?: BugsnagCore.BreadcrumbType[] | null;
appVersion?: string;
appType?: string;
endpoints?: { notify: string; sessions?: string };
Expand All @@ -23,10 +23,6 @@ declare module "@bugsnag/core" {
[key: string]: any;
// options for all bundled browser plugins
maxEvents?: number;
consoleBreadcrumbsEnabled?: boolean;
networkBreadcrumbsEnabled?: boolean;
navigationBreadcrumbsEnabled?: boolean;
interactionBreadcrumbsEnabled?: boolean;
collectUserIp?: boolean;
}
}
Expand Down
6 changes: 1 addition & 5 deletions packages/browser/types/test/fixtures/all-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ bugsnag({
enabledReleaseStages: [],
releaseStage: "production",
maxBreadcrumbs: 20,
autoBreadcrumbs: true,
enabledBreadcrumbTypes: ['manual','log','request'],
user: null,
metaData: null,
logger: undefined,
filters: ["foo",/bar/],
collectUserIp: true,
consoleBreadcrumbsEnabled: undefined,
interactionBreadcrumbsEnabled: undefined,
navigationBreadcrumbsEnabled: undefined,
networkBreadcrumbsEnabled: undefined,
maxEvents: 10
})
4 changes: 2 additions & 2 deletions packages/browser/types/test/fixtures/breadcrumb-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const bugsnagClient = bugsnag({
beforeSend: (event) => {
event.breadcrumbs.map(breadcrumb => {
console.log(breadcrumb.type)
console.log(breadcrumb.name)
console.log(breadcrumb.metaData)
console.log(breadcrumb.message)
console.log(breadcrumb.metadata)
console.log(breadcrumb.timestamp)
})
}
Expand Down
10 changes: 5 additions & 5 deletions packages/core/breadcrumb.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
const { isoDate } = require('./lib/es-utils')

class BugsnagBreadcrumb {
constructor (name = '[anonymous]', metaData = {}, type = 'manual', timestamp = isoDate()) {
constructor (message, metadata, type, timestamp = isoDate()) {
this.type = type
this.name = name
this.metaData = metaData
this.message = message
this.metadata = metadata
this.timestamp = timestamp
}

toJSON () {
return {
type: this.type,
name: this.name,
name: this.message,
timestamp: this.timestamp,
metaData: this.metaData
metaData: this.metadata
}
}
}
Expand Down
30 changes: 15 additions & 15 deletions packages/core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,21 @@ class BugsnagClient {
return this._sessionDelegate.startSession(this)
}

leaveBreadcrumb (name, metaData, type, timestamp) {
leaveBreadcrumb (message, metadata, type) {
if (!this._configured) throw new Error('client not configured')

// coerce bad values so that the defaults get set
name = name || undefined
type = typeof type === 'string' ? type : undefined
timestamp = typeof timestamp === 'string' ? timestamp : undefined
metaData = typeof metaData === 'object' && metaData !== null ? metaData : undefined
message = typeof message === 'string' ? message : ''
type = typeof type === 'string' ? type : 'manual'
metadata = typeof metadata === 'object' && metadata !== null ? metadata : {}

// if no name and no metaData, usefulness of this crumb is questionable at best so discard
if (typeof name !== 'string' && !metaData) return
// if no message, discard
if (!message) return

const crumb = new BugsnagBreadcrumb(name, metaData, type, timestamp)
// check the breadcrumb is the list of enabled types
if (!this.config.enabledBreadcrumbTypes || !includes(this.config.enabledBreadcrumbTypes, type)) return

const crumb = new BugsnagBreadcrumb(message, metadata, type)

// push the valid crumb onto the queue and maintain the length
this.breadcrumbs.push(crumb)
Expand Down Expand Up @@ -205,13 +207,11 @@ class BugsnagClient {
}

// only leave a crumb for the error if actually got sent
if (this.config.autoBreadcrumbs) {
this.leaveBreadcrumb(event.errorClass, {
errorClass: event.errorClass,
errorMessage: event.errorMessage,
severity: event.severity
}, 'error')
}
BugsnagClient.prototype.leaveBreadcrumb.call(this, event.errorClass, {
errorClass: event.errorClass,
errorMessage: event.errorMessage,
severity: event.severity
}, 'error')

if (originalSeverity !== event.severity) {
event._handledState.severityReason = { type: 'userCallbackSetSeverity' }
Expand Down
13 changes: 9 additions & 4 deletions packages/core/config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const { filter, reduce, keys, isArray, includes } = require('./lib/es-utils')
const { intRange, stringWithLength } = require('./lib/validators')

const BREADCRUMB_TYPES = ['navigation', 'request', 'process', 'log', 'user', 'state', 'error', 'manual']

module.exports.schema = {
apiKey: {
defaultValue: () => null,
Expand Down Expand Up @@ -68,10 +70,13 @@ module.exports.schema = {
message: 'should be a number ≤40',
validate: value => intRange(0, 40)(value)
},
autoBreadcrumbs: {
defaultValue: () => true,
message: 'should be true|false',
validate: (value) => typeof value === 'boolean'
enabledBreadcrumbTypes: {
defaultValue: () => BREADCRUMB_TYPES,
message: `should be null or a list of available breadcrumb types (${BREADCRUMB_TYPES.join(',')})`,
validate: value => value === null || (isArray(value) && reduce(value, (accum, maybeType) => {
if (accum === false) return accum
return includes(BREADCRUMB_TYPES, maybeType)
}, true))
},
user: {
defaultValue: () => null,
Expand Down
19 changes: 7 additions & 12 deletions packages/core/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ describe('@bugsnag/core/client', () => {
client.notify(new Error('foobar'))
expect(client.breadcrumbs.length).toBe(1)
expect(client.breadcrumbs[0].type).toBe('error')
expect(client.breadcrumbs[0].name).toBe('Error')
expect(client.breadcrumbs[0].metaData.stacktrace).toBe(undefined)
expect(client.breadcrumbs[0].message).toBe('Error')
expect(client.breadcrumbs[0].metadata.stacktrace).toBe(undefined)
// the error shouldn't appear as a breadcrumb for itself
expect(payloads[0].events[0].breadcrumbs.length).toBe(0)
})
Expand Down Expand Up @@ -431,8 +431,8 @@ describe('@bugsnag/core/client', () => {
client.leaveBreadcrumb('french stick')
expect(client.breadcrumbs.length).toBe(1)
expect(client.breadcrumbs[0].type).toBe('manual')
expect(client.breadcrumbs[0].name).toBe('french stick')
expect(client.breadcrumbs[0].metaData).toEqual({})
expect(client.breadcrumbs[0].message).toBe('french stick')
expect(client.breadcrumbs[0].metadata).toEqual({})
})

it('caps the length of breadcrumbs at the configured limit', () => {
Expand All @@ -447,27 +447,22 @@ describe('@bugsnag/core/client', () => {
expect(client.breadcrumbs.length).toBe(3)
client.leaveBreadcrumb('seedy farmhouse')
expect(client.breadcrumbs.length).toBe(3)
expect(client.breadcrumbs.map(b => b.name)).toEqual([
expect(client.breadcrumbs.map(b => b.message)).toEqual([
'medium sliced white hovis',
'pumperninkel',
'seedy farmhouse'
])
})

it('doesn’t add the breadcrumb if it didn’t contain anything useful', () => {
it('doesn’t add the breadcrumb if it didn’t contain a message', () => {
const client = new Client(VALID_NOTIFIER)
client.setOptions({ apiKey: 'API_KEY_YEAH' })
client.configure()
client.leaveBreadcrumb(undefined)
client.leaveBreadcrumb(null, { data: 'is useful' })
client.leaveBreadcrumb(null, {}, null)
client.leaveBreadcrumb(null, { t: 10 }, null, 4)
expect(client.breadcrumbs.length).toBe(3)
expect(client.breadcrumbs[0].type).toBe('manual')
expect(client.breadcrumbs[0].name).toBe('[anonymous]')
expect(client.breadcrumbs[0].metaData).toEqual({ data: 'is useful' })
expect(client.breadcrumbs[1].type).toBe('manual')
expect(typeof client.breadcrumbs[2].timestamp).toBe('string')
expect(client.breadcrumbs.length).toBe(0)
})

it('allows maxBreadcrumbs to be set to 0', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/core/test/config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,11 @@ describe('@bugsnag/core/config', () => {
})
})
})

describe('enabledBreadcrumbTypes', () => {
it('fails when a supplied value is not a valid breadcrumb type', () => {
const enabledBreadcrumbTypesValidator = config.schema.enabledBreadcrumbTypes.validate
expect(enabledBreadcrumbTypesValidator(['UNKNOWN_BREADCRUMB_TYPE'])).toBe(false)
})
})
})
5 changes: 2 additions & 3 deletions packages/core/types/breadcrumb.d.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
declare class Breadcrumb {
public name: string;
public metaData: object;
public message: string;
public metadata: object;
public type: string;
public timestamp: string;
constructor(name: string, metaData?: object, type?: string, timestamp?: string);
}

export default Breadcrumb;
2 changes: 1 addition & 1 deletion packages/core/types/client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ declare class Client {
opts?: common.NotifyOpts,
cb?: (err: any, event: Event) => void,
): void;
public leaveBreadcrumb(name: string, metaData?: any, type?: string, timestamp?: string): Client;
public leaveBreadcrumb(message: string, metadata?: { [key: string]: common.BreadcrumbMetadataValue }, type?: string): Client;
public startSession(): Client;
}

Expand Down
7 changes: 6 additions & 1 deletion packages/core/types/common.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Event from "./event";
export interface Config {
apiKey: string;
beforeSend?: BeforeSend | BeforeSend[];
autoBreadcrumbs?: boolean;
enabledBreadcrumbTypes?: BreadcrumbType[];
autoDetectErrors?: boolean;
autoDetectUnhandledRejections?: boolean;
appVersion?: string;
Expand Down Expand Up @@ -108,3 +108,8 @@ export type NotifiableError = Error
| { errorClass: string; errorMessage: string }
| { name: string; message: string }
| any;

type Primitive = boolean | string | number | undefined | null;
export type BreadcrumbMetadataValue = Primitive | Array<Primitive>;

export type BreadcrumbType = "error" | "log" | "manual" | "navigation" | "process" | "request" | "state" | "user";
9 changes: 1 addition & 8 deletions packages/expo/types/bugsnag.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ declare module "@bugsnag/core" {
interface Config {
apiKey?: string;
beforeSend?: BugsnagCore.BeforeSend | BugsnagCore.BeforeSend[];
autoBreadcrumbs?: boolean;
enabledBreadcrumbTypes?: BugsnagCore.BreadcrumbType[];
autoDetectErrors?: boolean;
autoDetectUnhandledRejections?: boolean;
appVersion?: string;
Expand All @@ -21,13 +21,6 @@ declare module "@bugsnag/core" {
filters?: Array<string | RegExp>;
// catch-all for any missing options
[key: string]: any;
// options for all bundled expo plugins
appStateBreadcrumbsEnabled?: boolean;
consoleBreadcrumbsEnabled?: boolean;
networkBreadcrumbsEnabled?: boolean;
navigationBreadcrumbsEnabled?: boolean;
connectivityBreadcrumbsEnabled?: boolean;
orientationBreadcrumbsEnabled?: boolean;
}
}

Expand Down
6 changes: 1 addition & 5 deletions packages/expo/types/test/fixtures/all-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ bugsnag({
enabledReleaseStages: ['zzz'],
releaseStage: "production",
maxBreadcrumbs: 20,
autoBreadcrumbs: true,
enabledBreadcrumbTypes: ['manual','log','request'],
user: null,
metaData: null,
logger: undefined,
filters: ["foo",/bar/],
collectUserIp: true,
consoleBreadcrumbsEnabled: undefined,
interactionBreadcrumbsEnabled: undefined,
navigationBreadcrumbsEnabled: undefined,
networkBreadcrumbsEnabled: undefined,
maxEvents: 10
})
4 changes: 2 additions & 2 deletions packages/expo/types/test/fixtures/breadcrumb-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const bugsnagClient = bugsnag({
beforeSend: (event) => {
event.breadcrumbs.map(breadcrumb => {
console.log(breadcrumb.type)
console.log(breadcrumb.name)
console.log(breadcrumb.metaData)
console.log(breadcrumb.message)
console.log(breadcrumb.metadata)
console.log(breadcrumb.timestamp)
})
}
Expand Down
4 changes: 2 additions & 2 deletions packages/node/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ 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 autoBreadcrumbs from the config schema
delete schema.autoBreadcrumbs
// remove enabledBreadcrumbTypes from the config schema
delete schema.enabledBreadcrumbTypes

const pluginSurroundingCode = require('@bugsnag/plugin-node-surrounding-code')
const pluginInProject = require('@bugsnag/plugin-node-in-project')
Expand Down
4 changes: 2 additions & 2 deletions packages/node/types/bugsnag.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ declare module "@bugsnag/core" {
interface Config {
apiKey: string;
beforeSend?: BugsnagCore.BeforeSend | BugsnagCore.BeforeSend[];
// autoBreadcrumbs?: boolean; // this option is disabled in node, see below
autoDetectErrors?: boolean;
autoDetectUnhandledRejections?: boolean;
appVersion?: string;
Expand All @@ -30,7 +29,8 @@ declare module "@bugsnag/core" {
agent?: any;
projectRoot?: string;
sendCode?: boolean;
autoBreadcrumbs?: void;
// breadcrumbs are disabled in Node
enabledBreadcrumbTypes?: void;
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-console-breadcrumbs/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# @bugsnag/plugin-console-breadcrumbs

This plugin adds the ability to record console method calls as breadcrumbs by monkey patching them. It defines a configuration option `consoleBreadcrumbsEnabled` which can be used to disable the functionality. It is included in the browser notifier.
This plugin adds the ability to record console method calls as breadcrumbs by monkey patching them. It is included in the browser notifier.

**Note:** enabling this plugin means that the line/col origin of console logs appear to come from within Bugsnag's code, so it is not recommended for use in dev environments.

Expand Down
14 changes: 2 additions & 12 deletions packages/plugin-console-breadcrumbs/console-breadcrumbs.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
const { map, reduce, filter } = require('@bugsnag/core/lib/es-utils')
const { map, reduce, filter, includes } = require('@bugsnag/core/lib/es-utils')

/*
* Leaves breadcrumbs when console log methods are called
*/
exports.init = (client) => {
const isDev = /^dev(elopment)?$/.test(client.config.releaseStage)

const explicitlyDisabled = client.config.consoleBreadcrumbsEnabled === false
const implicitlyDisabled = (client.config.autoBreadcrumbs === false || isDev) && client.config.consoleBreadcrumbsEnabled !== true
if (explicitlyDisabled || implicitlyDisabled) return
if (!client.config.enabledBreadcrumbTypes || !includes(client.config.enabledBreadcrumbTypes, 'log') || isDev) return

map(CONSOLE_LOG_METHODS, method => {
const original = console[method]
Expand Down Expand Up @@ -36,14 +34,6 @@ exports.init = (client) => {
})
}

exports.configSchema = {
consoleBreadcrumbsEnabled: {
defaultValue: () => undefined,
validate: (value) => value === true || value === false || value === undefined,
message: 'should be true|false'
}
}

if (process.env.NODE_ENV !== 'production') {
exports.destroy = () => CONSOLE_LOG_METHODS.forEach(method => {
if (typeof console[method]._restore === 'function') console[method]._restore()
Expand Down

0 comments on commit 8917cbc

Please sign in to comment.