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

V7: Simplify client configuration and store config privately #656

Merged
merged 2 commits into from
Nov 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
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 @@
- 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/650)
- Rename `beforeSend` -> `onError`, remove `event.ignore()` and refactor callback logic [#654](https://github.com/bugsnag/bugsnag-js/pull/654)
- 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)

## 6.4.3 (2019-10-21)

Expand Down
23 changes: 4 additions & 19 deletions packages/browser/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,13 @@ module.exports = (opts) => {
// handle very simple use case where user supplies just the api key as a string
if (typeof opts === 'string') opts = { apiKey: opts }

// support renamed/deprecated options

let warningMessage = ''

if (opts.endpoints && opts.endpoints.notify && !opts.endpoints.sessions) {
warningMessage += 'notify endpoint is set but sessions endpoint is not. No sessions will be sent.'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's now impossible to get into this state, as of #647

}

const bugsnag = new Client({ name, version, url })

bugsnag.setOptions(opts)
// configure a client with user supplied options
const bugsnag = new Client(opts, schema, { name, version, url })

// set delivery based on browser capability (IE 8+9 have an XDomainRequest object)
bugsnag.delivery(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)

// configure with user supplied options
// errors can be thrown here that prevent the lib from being in a useable state
bugsnag.configure(schema)

if (warningMessage) bugsnag._logger.warn(warningMessage)

// always-on browser-specific plugins
// add browser-specific plugins
bugsnag.use(pluginDevice)
bugsnag.use(pluginContext)
bugsnag.use(pluginRequest)
Expand All @@ -74,7 +59,7 @@ module.exports = (opts) => {

bugsnag._logger.debug('Loaded!')

return bugsnag.config.autoTrackSessions
return bugsnag._config.autoTrackSessions
? bugsnag.startSession()
: bugsnag
}
Expand Down
49 changes: 17 additions & 32 deletions packages/core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,14 @@ const LOG_USAGE_ERR_PREFIX = 'Usage error.'
const REPORT_USAGE_ERR_PREFIX = 'Bugsnag usage error.'

class BugsnagClient {
constructor (notifier) {
if (!notifier || !notifier.name || !notifier.version || !notifier.url) {
throw new Error('`notifier` argument is required')
}

constructor (configuration, schema = config.schema, notifier) {
// notifier id
this.notifier = notifier

// configure() should be called before notify()
this._configured = false
this._notifier = notifier

// intialise opts and config
this._opts = {}
this.config = {}
this._opts = configuration
this._config = {}
this._schema = schema

// // i/o
this._delivery = { sendSession: () => {}, sendEvent: () => {} }
Expand All @@ -51,18 +45,16 @@ class BugsnagClient {
this.BugsnagBreadcrumb = BugsnagBreadcrumb
this.BugsnagSession = BugsnagSession

this._extractConfiguration()

const self = this
const notify = this.notify
this.notify = function () {
return notify.apply(self, arguments)
}
}

setOptions (opts) {
this._opts = { ...this._opts, ...opts }
}

configure (partialSchema = config.schema) {
_extractConfiguration (partialSchema = this._schema) {
const conf = config.mergeDefaults(this._opts, partialSchema)
const validity = config.validate(conf, partialSchema)

Expand All @@ -77,16 +69,13 @@ class BugsnagClient {
if (conf.logger) this.logger(conf.logger)

// merge with existing config
this.config = { ...this.config, ...conf }

this._configured = true
this._config = { ...this._config, ...conf }

return this
}

use (plugin, ...args) {
if (!this._configured) throw new Error('client not configured')
if (plugin.configSchema) this.configure(plugin.configSchema)
if (plugin.configSchema) this._extractConfiguration(plugin.configSchema)
const result = plugin.init(this, ...args)
// JS objects are not the safest way to store arbitrarily keyed values,
// so bookend the key with some characters that prevent tampering with
Expand Down Expand Up @@ -124,8 +113,6 @@ class BugsnagClient {
}

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

// coerce bad values so that the defaults get set
message = typeof message === 'string' ? message : ''
type = typeof type === 'string' ? type : 'manual'
Expand All @@ -135,22 +122,20 @@ class BugsnagClient {
if (!message) return

// check the breadcrumb is the list of enabled types
if (!this.config.enabledBreadcrumbTypes || !includes(this.config.enabledBreadcrumbTypes, type)) return
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)
if (this.breadcrumbs.length > this.config.maxBreadcrumbs) {
this.breadcrumbs = this.breadcrumbs.slice(this.breadcrumbs.length - this.config.maxBreadcrumbs)
if (this.breadcrumbs.length > this._config.maxBreadcrumbs) {
this.breadcrumbs = this.breadcrumbs.slice(this.breadcrumbs.length - this._config.maxBreadcrumbs)
}

return this
}

notify (error, onError, cb = () => {}) {
if (!this._configured) throw new Error('client not configured')

// releaseStage can be set via config.releaseStage or client.app.releaseStage
const releaseStage = inferReleaseStage(this)

Expand All @@ -174,7 +159,7 @@ class BugsnagClient {
}

// exit early if events should not be sent on the current releaseStage
if (this.config.enabledReleaseStages.length > 0 && !includes(this.config.enabledReleaseStages, releaseStage)) {
if (this._config.enabledReleaseStages.length > 0 && !includes(this._config.enabledReleaseStages, releaseStage)) {
this._logger.warn('Event not sent due to releaseStage/enabledReleaseStages configuration')
return cb(null, event)
}
Expand All @@ -187,7 +172,7 @@ class BugsnagClient {
this._logger.error(err)
}

const callbacks = [].concat(onError).concat(this.config.onError)
const callbacks = [].concat(onError).concat(this._config.onError)
runCallbacks(callbacks, event, onCallbackError, (err, shouldSend) => {
if (err) onCallbackError(err)

Expand All @@ -208,8 +193,8 @@ class BugsnagClient {
}

this._delivery.sendEvent({
apiKey: event.apiKey || this.config.apiKey,
notifier: this.notifier,
apiKey: event.apiKey || this._config.apiKey,
notifier: this._notifier,
events: [event]
}, (err) => cb(err, event))
})
Expand Down
5 changes: 2 additions & 3 deletions packages/core/lib/clone-client.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
module.exports = (client) => {
const clone = new client.BugsnagClient(client.notifier)
clone.configure({})
const clone = new client.BugsnagClient({}, {}, client._notifier)

// changes to these properties should be reflected in the original client
clone.config = client.config
clone._config = client._config
clone.app = client.app
clone.context = client.context
clone.device = client.device
Expand Down
2 changes: 1 addition & 1 deletion packages/core/lib/infer-release-stage.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module.exports = client =>
client.app && typeof client.app.releaseStage === 'string'
? client.app.releaseStage
: client.config.releaseStage
: client._config.releaseStage