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

Breadcrumbs #149

Merged
merged 36 commits into from
Jul 22, 2016
Merged

Breadcrumbs #149

merged 36 commits into from
Jul 22, 2016

Conversation

foxyblocks
Copy link

@foxyblocks foxyblocks commented Jul 7, 2016

The UI isn't yet ready on Bugsnag.com but here is a screenshot:

breadcrumbs1

Bugsnag.leaveBreadcrumb(value, [metaData])

Add a breadcrumb to the array of breadcrumbs to be sent to Bugsnag when the next exception occurs

Bugsnag.leaveBreadcrumb("Increased Volume")

Each breadcrumb can also have additional diagnostic metadata added.

Bugsnag.leaveBreadcrumb('Increased Volume', {
  from: 5,
  to: 11
})

The last 25 breadcrumbs get stored and sent along with all payloads to bugsnag.

Automatic breadcrumbs

Includes automatic creation of breadcrumbs for certain events.

  • Clicks
  • Errors
  • Console
    • log
    • warn
    • error
  • Navigation
    • Page load
    • DOMContentLoaded
    • Page hide
    • Page show
    • popstate
    • history.pushState
    • history.replaceState

New configuration options

To disable all automatic breadcrumbs.

Bugsnag.autoBreadcrumbs = false;

You can also disable any individual automatic breadcrumb groups:

Bugsnag.autoBreadcrumbsClicks = false;
Bugsnag.autoBreadcrumbsErrors = false;
Bugsnag.autoBreadcrumbsConsole = false;
Bugsnag.autoBreadcrumbsNavigation = false;

If you only want to enable a single type of automatic breadcrumb you can disable all of them and then
enable the individual group you want:

// disable all automatic breadcrumbs except click events
Bugsnag.autoBreadcrumbs = false;
Bugsnag.autoBreadcrumbsClicks = true;

TODO

  • Add typescript header for leaveBreadcrumb
  • Address remaining inline TODO comments in the diff

return;
}

window.addEventListener("click", function(event) {

Choose a reason for hiding this comment

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

Would we need to do feature detection here to prevent this from throwing an error in older IE versions?

Copy link
Author

Choose a reason for hiding this comment

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

Does the polyfill further down for addEventListener allow this?

Choose a reason for hiding this comment

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

I think we only polyfill some minor behavior differences on different versions of addEventListener, so if it's not already there in a browser I don't know that we'd want to rely on bugsnag-js to polyfill it. As a third party it seems nicer/safer for us to not add new things to window if they're not there already, so I'd vote for us just skipping any automatic breadcrumbs that rely on events in browsers without addEventListener. (We could consider adding attachEvent support if a lot of people request it, I guess?)

Copy link
Author

Choose a reason for hiding this comment

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

👍

@snmaynard
Copy link
Contributor

What effect does this have on the compressed and minified js size?

@foxyblocks
Copy link
Author

Before:

Size:    26810
Ugly:     6922
Gzip:     3072

After:

Size:    36585
Ugly:    10196
Gzip:     4068

@apsoto
Copy link

apsoto commented Jul 8, 2016

Looks useful, but I use bugsnag explicitly because it works on embedded browsers (Opera TV). Please make sure this does the appropriate checks to prevent problems on non-mainstream browsers.

@foxyblocks
Copy link
Author

Looks useful, but I use bugsnag explicitly because it works on embedded browsers (Opera TV). Please make sure this does the appropriate checks to prevent problems on non-mainstream browsers.

@apsoto Our plan is to do browser feature detection around the new features in order to degrade gracefully. The leaveBreadcrumb() function itself should have the same browser support that the rest of the library has, but some of the automatic breadcrumbs won't work in older browsers.

We are also planning on releasing this as a new major version (3.0), so you will be able to test it before deciding to upgrade. Any bug reports about Opera TV specific issues will be greatly appreciated.

@@ -252,6 +503,66 @@
}
}

// Compare if two objects are equal.
// TODO check if this would fail if the properties are traveresed in different orders
Copy link

Choose a reason for hiding this comment

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

Answer: Yes, it does fail 😛

@@ -504,7 +504,6 @@
}

// Compare if two objects are equal.
// TODO check if this would fail if the properties are traveresed in different orders
function isEqual(obj1, obj2) {
serialize(obj1) === serialize(obj2);
Copy link

Choose a reason for hiding this comment

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

Me thinks there also might be a return missing? Unless you're using it for testing right now, in which case I'll get lost and quit bugging you :P

Copy link
Author

Choose a reason for hiding this comment

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

Doh! good catch

By default, we automatically create breadcrumbs for the following types of events:

- **Clicks**
- **Errors**

Choose a reason for hiding this comment

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

Maybe it's worth saying "Uncaught errors" for clarity?

@kattrali kattrali merged commit 4e4d5f9 into master Jul 22, 2016
@kattrali kattrali deleted the breadcrumbs branch July 22, 2016 22:33
@kattrali
Copy link
Contributor

🎉

@foxyblocks
Copy link
Author

@kattrali
Copy link
Contributor

...That is a mesmerizing GIF.

@sandstrom
Copy link

@wordofchristian Great feature, just implemented rc1 and it works well.

Some thoughts:

  1. UI for the breadcrumbs could be improved, i.e. the console output is needlessly verbose now. Instead of using the severity and message keywords in the UI it should simply use the icon + the message + severity string.
  2. Great if we could manually create the built-in things, e.g. console messages. I got it working by looking at the source, but I assume those are private APIs that could change? or are they not?

@foxyblocks
Copy link
Author

@sandstrom Yes we are planning on documenting those types at some point. We didn't initially so as to keep the API and documentation simpler. You should be able to continue using it the way you have been, and it will continue to work.

Bugsnag.leaveBreadcrumb({
  type: "error|navigation|user|log|manual",
  name: "Anything",
  metaData: {
    anyKey: "any value",
    foo: "bar",
    biz: "baz",
  }
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants