-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
Package + Version
@sentry/react@5.25.0@sentry/tracing@5.25.0
Description
addBreadcrumb is defined in several places:
- Hub
- Hub scope
- Minimal alias for hub (aka,
Sentry.addBreadcrumb).
When addBreadcrumb is called on hub, it will normalize the max breadcrumbs, defaulting to 100, and never allowing more than 100. It then calls scope.addBreadcrumb with this normalized max. This means that when Sentry.addBreadcrumb is used, we can know that our breadcrumbs are trimmed and don't grow out of control.
When scope.addBreadcrumb is called, however, it does not fall back to a default / max as this is defined in the hub. This is a huge issue in some cases. Take @sentry/react redux implementation, for example. As you can see, this does not call addBreadcrumb on the hub, and does not pass in maxBreadcrumbs. As such, it is relatively easy to get into a state where this represents a memory leak. In any case where scope.addBreadcrumb is called without maxBreadcrumbs, and getCurrentHub().addBreadcrumb is not called, breadcrumbs can grow internally forever.
Additionally, although there aren't explicit examples in the docs using scope.addBreadcrumb (that I could find), it does explain that scopes contain breadcrumbs, and the types make it clear that addBreadcrumb is on the scope when using TypeScript.
I think the minimal fix for this is simply to bring MAX_BREADCRUMBS into scope and do the maxBreadcrumbs normalization there instead of in hub. This way, at the very least, there is an upper limit on the number of breadcrumbs that can be added to scope.