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: Remove non-public methods from client interface #659

Merged
merged 3 commits into from
Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -13,6 +13,7 @@
- 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)
- Remove non-public methods from `Client` interface: `logger()`, `delivery()` and `sessionDelegate()` [#659](https://github.com/bugsnag/bugsnag-js/pull/659)

## 6.4.3 (2019-10-21)

Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ module.exports = (opts) => {
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)
bugsnag._setDelivery(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)

// add browser-specific plugins
bugsnag.use(pluginDevice)
Expand Down
14 changes: 2 additions & 12 deletions packages/core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class BugsnagClient {
if (conf.appType) this.app.type = conf.appType
if (conf.metadata) this._metadata = conf.metadata
if (conf.user) this._user = conf.user
if (conf.logger) this.logger(conf.logger)
if (conf.logger) this._logger = conf.logger

// merge with existing config
this._config = { ...this._config, ...conf }
Expand Down Expand Up @@ -111,21 +111,11 @@ class BugsnagClient {
return this._plugins[`~${name}~`]
}

delivery (d) {
_setDelivery (d) {
this._delivery = d(this)
return this
bengourley marked this conversation as resolved.
Show resolved Hide resolved
}

logger (l, sid) {
this._logger = l
return this
}

sessionDelegate (s) {
this._sessionDelegate = s
return this
}

startSession () {
if (!this._sessionDelegate) {
this._logger.warn('No session implementation is installed')
Expand Down
54 changes: 27 additions & 27 deletions packages/core/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('@bugsnag/core/client', () => {
expect(msg).toBeTruthy()
done()
}
client.logger({ debug: log, info: log, warn: log, error: log })
client._logger = { debug: log, info: log, warn: log, error: log }
client._logger.debug('hey')
})
it('can supply a different logger via config', done => {
Expand Down Expand Up @@ -78,7 +78,7 @@ describe('@bugsnag/core/client', () => {
describe('notify()', () => {
it('delivers an error event', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: payload => {
expect(payload).toBeTruthy()
expect(Array.isArray(payload.events)).toBe(true)
Expand All @@ -93,7 +93,7 @@ describe('@bugsnag/core/client', () => {

it('supports setting severity via callback', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
expect(payload).toBeTruthy()
expect(Array.isArray(payload.events)).toBe(true)
Expand All @@ -110,7 +110,7 @@ describe('@bugsnag/core/client', () => {

it('supports preventing send by returning false', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
fail('sendEvent() should not be called')
}
Expand All @@ -133,7 +133,7 @@ describe('@bugsnag/core/client', () => {
onErrorSpy
]
})
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
expect(payload.events[0].errorMessage).toBe('oh no!')
expect(onErrorSpy).toHaveBeenCalledTimes(1)
Expand All @@ -146,7 +146,7 @@ describe('@bugsnag/core/client', () => {

it('supports preventing send with enabledReleaseStages', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['qa'] })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
fail('sendEvent() should not be called')
}
Expand All @@ -160,7 +160,7 @@ describe('@bugsnag/core/client', () => {

it('supports setting releaseStage via config.releaseStage', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', releaseStage: 'staging', enabledReleaseStages: ['production'] })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
fail('sendEvent() should not be called')
}
Expand All @@ -174,7 +174,7 @@ describe('@bugsnag/core/client', () => {

it('supports setting releaseStage via client.app.releaseStage', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['production'] })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
fail('sendEvent() should not be called')
}
Expand All @@ -189,7 +189,7 @@ describe('@bugsnag/core/client', () => {

it('includes releaseStage in event.app', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['staging'] })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
expect(payload.events[0].app.releaseStage).toBe('staging')
done()
Expand All @@ -201,7 +201,7 @@ describe('@bugsnag/core/client', () => {

it('includes releaseStage in event.app when set via config', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['staging'], releaseStage: 'staging' })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
expect(payload.events[0].app.releaseStage).toBe('staging')
done()
Expand All @@ -212,7 +212,7 @@ describe('@bugsnag/core/client', () => {

it('prefers client.app.releaseStage over config.releaseStage', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['testing'], releaseStage: 'staging' })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
expect(payload.events[0].app.releaseStage).toBe('testing')
done()
Expand All @@ -224,7 +224,7 @@ describe('@bugsnag/core/client', () => {

it('populates client.app.version if config.appVersion is supplied', done => {
const client = new Client({ apiKey: 'API_KEY_YEAH', appVersion: '1.2.3' })
client.delivery(client => ({
client._setDelivery(client => ({
sendEvent: (payload) => {
expect(payload.events[0].app.version).toBe('1.2.3')
done()
Expand All @@ -236,7 +236,7 @@ describe('@bugsnag/core/client', () => {
it('can handle all kinds of bad input', () => {
const payloads = []
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))

client.notify(undefined)
client.notify(null)
Expand All @@ -261,7 +261,7 @@ describe('@bugsnag/core/client', () => {
it('supports { name, message } usage', () => {
const payloads = []
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify({ name: 'UnknownThing', message: 'found a thing that couldn’t be dealt with' })

expect(payloads.length).toBe(1)
Expand All @@ -274,7 +274,7 @@ describe('@bugsnag/core/client', () => {
it('leaves a breadcrumb of the error', () => {
const payloads = []
const client = new Client({ apiKey: 'API_KEY_YEAH' })
client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify(new Error('foobar'))
expect(client.breadcrumbs.length).toBe(1)
expect(client.breadcrumbs[0].type).toBe('error')
Expand All @@ -295,7 +295,7 @@ describe('@bugsnag/core/client', () => {

it('should call the callback (success)', done => {
const client = new Client({ apiKey: 'API_KEY' })
client.delivery(client => ({
client._setDelivery(client => ({
sendSession: () => {},
sendEvent: (payload, cb) => cb(null)
}))
Expand All @@ -309,7 +309,7 @@ describe('@bugsnag/core/client', () => {

it('should call the callback (err)', done => {
const client = new Client({ apiKey: 'API_KEY' })
client.delivery(client => ({
client._setDelivery(client => ({
sendSession: () => {},
sendEvent: (payload, cb) => cb(new Error('flerp'))
}))
Expand All @@ -324,7 +324,7 @@ describe('@bugsnag/core/client', () => {

it('should call the callback even if the event doesn’t send (enabledReleaseStages)', done => {
const client = new Client({ apiKey: 'API_KEY', enabledReleaseStages: ['production'], releaseStage: 'development' })
client.delivery(client => ({
client._setDelivery(client => ({
sendSession: () => {},
sendEvent: (payload, cb) => cb(null)
}))
Expand All @@ -338,7 +338,7 @@ describe('@bugsnag/core/client', () => {

it('should call the callback even if the event doesn’t send (onError)', done => {
const client = new Client({ apiKey: 'API_KEY', onError: () => false })
client.delivery(client => ({
client._setDelivery(client => ({
sendSession: () => {},
sendEvent: (payload, cb) => cb(null)
}))
Expand All @@ -352,7 +352,7 @@ describe('@bugsnag/core/client', () => {

it('should attach the original error to the event object', done => {
const client = new Client({ apiKey: 'API_KEY', onError: () => false })
client.delivery(client => ({
client._setDelivery(client => ({
sendSession: () => {},
sendEvent: (payload, cb) => cb(null)
}))
Expand Down Expand Up @@ -418,40 +418,40 @@ describe('@bugsnag/core/client', () => {
it('calls the provided the session delegate and return delegate’s return value', () => {
const client = new Client({ apiKey: 'API_KEY' })
let ret
client.sessionDelegate({
client._sessionDelegate = {
startSession: c => {
expect(c).toBe(client)
ret = {}
return ret
}
})
}
expect(client.startSession()).toBe(ret)
})

it('calls warns if a session delegate is not provided', (done) => {
const client = new Client({ apiKey: 'API_KEY' })
client.logger({
client._logger = {
debug: () => {},
info: () => {},
warn: (...args) => {
expect(args[0]).toMatch(/^No session/)
done()
},
error: () => {}
})
}
client.startSession()
})

it('tracks error counts using the session delegate and sends them in error payloads', (done) => {
const client = new Client({ apiKey: 'API_KEY' })
let i = 0
client.sessionDelegate({
client._sessionDelegate = {
startSession: (client) => {
client._session = new Session()
return client
}
})
client.delivery(client => ({
}
client._setDelivery(client => ({
sendSession: () => {},
sendEvent: (payload, cb) => {
if (++i < 10) return
Expand Down
3 changes: 0 additions & 3 deletions packages/core/types/client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ declare class Client {

public use(plugin: common.Plugin, ...args: any[]): Client;
public getPlugin(name: string): any;
public delivery(delivery: common.Delivery): Client;
public logger(logger: common.Logger): Client;
public sessionDelegate(sessionDelegate: common.SessionDelegate): Client;
public notify(
error: common.NotifiableError,
onError?: common.OnError,
Expand Down
16 changes: 0 additions & 16 deletions packages/core/types/common.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,6 @@ export interface ConfigSchema {
[key: string]: ConfigSchemaEntry;
}

export interface Delivery {
name: string;
sendEvent: (
logger: Logger,
config: any,
event: EventPayload,
cb?: (e: Error | null, resText: string) => void,
) => void;
sendSession: (
logger: Logger,
config: any,
event: SessionPayload,
cb?: (e: Error | null, resText: string) => void,
) => void;
}

export interface Logger {
debug: (...args: any[]) => void;
info: (...args: any[]) => void;
Expand Down
2 changes: 1 addition & 1 deletion packages/expo/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ module.exports = (opts) => {

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

bugsnag.delivery(delivery)
bugsnag._setDelivery(delivery)

plugins.forEach(pl => {
switch (pl.name) {
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module.exports = (opts, userPlugins = []) => {

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

bugsnag.delivery(delivery)
bugsnag._setDelivery(delivery)

plugins.forEach(pl => bugsnag.use(pl))

Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-browser-context/test/context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('plugin: context', () => {
const payloads = []
client.use(plugin, window)

client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify(new Error('noooo'))

expect(payloads.length).toEqual(1)
Expand All @@ -30,7 +30,7 @@ describe('plugin: context', () => {

client.context = 'something else'

client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify(new Error('noooo'))

expect(payloads.length).toEqual(1)
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-browser-device/test/device.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('plugin: device', () => {

expect(client._config.onError.length).toBe(1)

client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify(new Error('noooo'))

const ISO_8601 = /^\d{4}(-\d\d(-\d\d(T\d\d:\d\d(:\d\d)?(\.\d+)?(([+-]\d\d:\d\d)|Z)?)?)?)?$/i
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-browser-request/test/request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('plugin: request', () => {
const payloads = []
client.use(plugin, window)

client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify(new Error('noooo'))

expect(payloads.length).toEqual(1)
Expand All @@ -26,7 +26,7 @@ describe('plugin: request', () => {

client.request = { url: 'foobar' }

client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
client.notify(new Error('noooo'))

expect(payloads.length).toEqual(1)
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-browser-session/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const { includes } = require('@bugsnag/core/lib/es-utils')
const inferReleaseStage = require('@bugsnag/core/lib/infer-release-stage')

module.exports = {
init: client => client.sessionDelegate(sessionDelegate)
init: client => { client._sessionDelegate = sessionDelegate }
}

const sessionDelegate = {
Expand Down
Loading