Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

capture breadcrumbs from global context if not in active context #252

Conversation

vineethawal
Copy link
Contributor

No description provided.

@benvinegar
Copy link
Contributor

@LewisJEllis – any reason not to merge this?

@LewisJEllis
Copy link
Contributor

LewisJEllis commented Jan 19, 2017

This was a somewhat intentional decision, though perhaps not deeply considered. My concern here is that a slightly clueless user might set up raven and just stop at:

var Raven = require('raven');
Raven.config(dsn).install({ autoBreadcrumbs: true });

without using any contexts, the express middleware, or whatever for breadcrumbs to be scoped to. A simple test script would then appear to show breadcrumbs being gathered correctly, but really all breadcrumbs are going into the global context breadcrumb buffer. In a long-running production app, we'd then get breadcrumbs from all sorts of stuff other than the current request (since it all just goes to the global buffer), and the user might look at the dashboard and see lots of (irrelevant) breadcrumbs and not be sure what's going on. This could be even more likely once autobreadcrumbs are on by default.

I'm of the opinion that if you want breadcrumbs, you should be using contexts, period. That doesn't necessarily have to be the case, but I'm not sure what the benefit would be. I'm open to either suggestions for how to avoid this confusing user experience, or to arguments that this isn't really that confusing and collecting the global context breadcrumbs is somehow rather useful.

The interesting point this brings to mind, of course, is that we do capture breadcrumbs into the global context's breadcrumb buffer if we're not in any other context. If we never actually include those with a captured exception, there's no point to doing so.

My proposed action items are then:

  • Stop bothering to capture breadcrumbs at all when in the global context
  • Make it more clear in docs that breadcrumbs require being in a context

but I'm open to discussion/ideas/motivating use cases for this PR's potential behavior change.

@LewisJEllis
Copy link
Contributor

LewisJEllis commented Feb 3, 2017

Me, earlier:

I'm not sure what the benefit would be. I'm open to ... arguments that ... collecting the global context breadcrumbs is somehow rather useful.

The above-alluded-to potential benefit/usefulness of capturing breadcrumbs from the global context has been found in #265 with the Lambda + Promises use case. I'm going to add tests to this and then merge it, make some other small tweaks on warning messages, and then release a patch version.

@LewisJEllis
Copy link
Contributor

Addressed in #267

@LewisJEllis LewisJEllis closed this Feb 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants