sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Stack parsers use both `?` and `<anonymous>` to denote anonymous functions

Open timfish opened this issue 3 years ago • 9 comments

Chrome/v8 stack frames use ? to denote anonymous functions but we also use <anonymous> in a few places.

https://github.com/getsentry/sentry-javascript/blob/78395c6c6f2cf7b9a8afe907c6396d911b83d3b6/packages/utils/src/stacktrace.ts#L75

https://github.com/getsentry/sentry-javascript/blob/78395c6c6f2cf7b9a8afe907c6396d911b83d3b6/packages/utils/src/stacktrace.ts#L80

timfish avatar Aug 04 '22 13:08 timfish

Hey - you're right that this is kind of a weird inconsistency. @kamilogorek any ideas on why this happens?

AbhiPrasad avatar Aug 05 '22 16:08 AbhiPrasad

If we change this, will it impact fingerprinting?

timfish avatar Aug 05 '22 18:08 timfish

Mostly likely yes - we'll need to understand the grouping impact, but we can get folks more involved there (and run some queries in Sentry to determine possible impact).

AbhiPrasad avatar Aug 05 '22 19:08 AbhiPrasad

@kamilogorek any ideas on why this happens?

Historical reasons. It was called <anonymous> (the same value as anonymous URLs/files in some old browsers) since raven SDK, and we kept it like that. ? comes directly from Tracekit, which was also not changed since the old SDK, as the grouping strategy back then did not group them together afair. Changing this would cause some new issues to pop up.

Now, this will not affect any grouping (at least for one version of it), due to those frames being skipped, see: https://github.com/getsentry/sentry/blob/46d9c2aa672b8abe2e26e52c60e9ffc32eba9c23/src/sentry/grouping/strategies/newstyle.py#L394-L398

Also original references: https://github.com/getsentry/sentry-javascript/blob/7b664ff3a828fe59e32ccc182d5b4bb449dc9402/packages/raven-js/src/raven.js#L1122 https://github.com/getsentry/sentry-javascript/blob/7b664ff3a828fe59e32ccc182d5b4bb449dc9402/packages/raven-js/vendor/TraceKit/tracekit.js#L29

kamilogorek avatar Aug 08 '22 14:08 kamilogorek

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

github-actions[bot] avatar Aug 30 '22 00:08 github-actions[bot]

I'm going to submit a PR to change everything to ?...

timfish avatar Aug 31 '22 15:08 timfish

yeah that sounds good to me - will be more bundle size efficient. Maybe we should also write this in the developer docs somewhere.

AbhiPrasad avatar Aug 31 '22 15:08 AbhiPrasad

This could potentially be a breaking change since customers might be using RewriteFrames and expecting some specific anonymous frames?

Seems highly unlikely anyone is doing this but worth considering.

timfish avatar Aug 31 '22 15:08 timfish

How about we just backlog for the next major bump then? I don't mind tabling this patch until then since we treat it identically for grouping strategies.

AbhiPrasad avatar Aug 31 '22 18:08 AbhiPrasad

resolved with https://github.com/getsentry/sentry-javascript/pull/10732

AbhiPrasad avatar Feb 21 '24 02:02 AbhiPrasad