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

feat(browser): Integrate maxEvents functionality with new interface #727

Merged
merged 1 commit into from
Feb 5, 2020
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
2 changes: 1 addition & 1 deletion packages/browser/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const Bugsnag = {
}
}

map(keys(Client.prototype), (m) => {
map(['resetEventCount'].concat(keys(Client.prototype)), (m) => {
if (/^_/.test(m)) return
Bugsnag[m] = function () {
if (!Bugsnag._client) return console.log(`Bugsnag.${m}() was called before Bugsnag.start()`)
Expand Down
3 changes: 3 additions & 0 deletions packages/core/types/client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ declare class Client {
public use(plugin: Plugin, ...args: any[]): Client;
public getPlugin(name: string): any;

// implemented on the browser notifier only
public resetEventCount?(): void;
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 seemed like the most reasonable way to implement this. If I did something like class BrowserClient extends Client to add the method only for the browser, it upsets the "universal" interface. Specifically it upset the Angular plugin which does import { Client } from '@bugsnag/js' – it doesn't know if Client should be a Client or BrowserClient since it can be run in Node or Browser.


// access to internal classes
public Breadcrumb: typeof Breadcrumb;
public Event: typeof Event;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ const wrapHistoryFn = (client, target, fn, win) => {
const orig = target[fn]
target[fn] = (state, title, url) => {
client.leaveBreadcrumb(`History ${fn}`, stateChangeToMetadata(win, state, title, url), 'navigation')
// if throttle plugin is in use, refresh the event sent count
if (typeof client.refresh === 'function') client.refresh()
// if throttle plugin is in use, reset the event sent count
if (typeof client.resetEventCount === 'function') client.resetEventCount()
// if the client is operating in auto session-mode, a new route should trigger a new session
if (client._config.autoTrackSessions) client.startSession()
// Internet Explorer will convert `undefined` to a string when passed, causing an unintended redirect
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-simple-throttle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

This plugin adds a safety mechanism to prevent too many events being sent to Bugsnag, saving client bandwidth and error quotas using a simple threshold – i.e. do not send more than `n` events. It defines a configuration option `maxEvents` which can be used to customize the behaviour. It is included in the browser notifier.

**Note:** to support long-lived browser applications, this plugin defines a `refresh()` function which gets called when the URL of the page changes. This resets the count, allowing new events to be sent.
**Note:** to support long-lived browser applications, this plugin defines a `resetEventCount()` method on the client which gets called when the URL of the page changes. This resets the count, allowing new events to be sent.

## License
MIT
2 changes: 1 addition & 1 deletion packages/plugin-simple-throttle/throttle.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports = {
n++
})

client.refresh = () => { n = 0 }
client.resetEventCount = () => { n = 0 }
},
configSchema: {
maxEvents: {
Expand Down