Skip to content

Conversation

@kamilogorek
Copy link
Contributor

Sentry.configureScope(scope => {
  for (let i = 0; i < 1000; i++) {
    scope.addBreadcrumb({ message: 'hi' })
  }
})

This would store 1000 crumbs, despite having hard limit of 100, as this limit was stored in the Hub, and not Scope where it should. cc @timfish

fixes #2963
fixes getsentry/sentry-electron#205
fixes getsentry/sentry-electron#277

@kamilogorek kamilogorek requested review from a team, ahmedetefy and iker-barriocanal and removed request for a team June 15, 2021 08:56
for (let i = 0; i < 10; i++) {
scope.addBreadcrumb({ message: 'test' }, 5);
}
expect((scope as any)._breadcrumbs).toHaveLength(5);
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also specifically check for which 5 breadcrumbs we want: last, first, etc. I think it should be last N, right?

* `maxBreadcrumbs` option cannot be higher than this value.
*/
const MAX_BREADCRUMBS = 100;

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this could break downstream dependencies. Could we leave it in?

@github-actions
Copy link
Contributor

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.98 KB (+0.06% 🔺)
@sentry/browser - Webpack 21.85 KB (+0.05% 🔺)
@sentry/react - Webpack 21.89 KB (+0.05% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.37 KB (+0.06% 🔺)

@kamilogorek kamilogorek merged commit c4fc9ae into master Jun 15, 2021
@kamilogorek kamilogorek deleted the scope-breadcrumbs-limit branch June 15, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants