Rethink debouncing of click handlers

[lforst]

Looking at https://github.com/getsentry/sentry-javascript/issues/9204 @mydea and I started to question whether it is a good idea even to have the functionality that debounces the triggering of our internal event handler for "similar" click events.

What am I talking about?

https://github.com/getsentry/sentry-javascript/blob/535d094bcdee68f7b742cc7d524510d73c667baf/packages/utils/src/instrument.ts#L485-L505

For example, a user rage clicks 10 times on an element with a throwing event handler. The SDK will only create 1 error, 1 breadcrumb, one of whatever we attach our internal event handler to.

As a user, I would not expect this to be the behavior and we should reconsider this.

Of course, this comes with a lot of product impact (more errors, more errors in replay, different breadcrumbs, who knows what else) so we declare this a breaking change.

[Lms24]

I'm in favour of removing the debouncing logic when considering breadcrumbs and replays. For errors, our Dedupe integration should take care of filtering duplicated errors, assuming the throwing handler always throws the same error. So in a way, we already have a mechanism to protect against duplicate errors. However, if the throwing handler actually throws distinct errors, we'd right now even suppress errors that could be valuable for users.

[lforst]

The one thing I am a bit hesitant about is: Are the errors useful to get? If I get a bazillion errors from rage clicks I probably don't care about the individual errors. I probably just care that I get the error.

In general, I am leaning towards removing the debouncing logic though. Gets rid of complexity. Makes SDK more transparent.

[Lms24]

If I get a bazillion errors from rage clicks I probably don't care about the individual errors

It's a good point but I'm not sure how likely this scenario is as long as Dedupe is active. Sure, not every case is covered by the integration but maybe enough? IIRC we talked about keeping a log of the last N errors and deduping them to cover error sequences like A B A B A B ...

[AbhiPrasad]

It's a good point but I'm not sure how likely this scenario is as long as Dedupe is active. Sure, not every case is covered by the integration but maybe enough? IIRC we talked about keeping a log of the last N errors and deduping them to cover error sequences like A B A B A B ...

Let's not purely try to solve the debouncing problem from the SDK side for these cases. Part of this is also a quota management issue, so we need to get some product advice on this.

[lforst]

I think I changed my opinion on this. I think we should keep debouncing click handler errors. However, I think we have weird behaviour where we don't create click breadcrumbs if an event is captured as part of the click handler and the click happens multiple times in a row.