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

fix(browser): Don't stomp xhr function names

Open lforst opened this issue 1 year ago • 1 comments

Fixes https://github.com/getsentry/sentry-javascript/issues/12208

lforst avatar May 24 '24 11:05 lforst

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 22.52 KB +0.08% +17 B 🔺
@sentry/browser (incl. Tracing) 34.85 KB +0.02% +7 B 🔺
@sentry/browser (incl. Tracing, Replay) 71.21 KB +0.02% +11 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 64.48 KB +0.02% +12 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 75.57 KB +0.03% +18 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 88.29 KB +0.01% +8 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 90.13 KB +0.01% +5 B 🔺
@sentry/browser (incl. metrics) 26.83 KB +0.06% +16 B 🔺
@sentry/browser (incl. Feedback) 39.59 KB +0.06% +23 B 🔺
@sentry/browser (incl. sendFeedback) 27.18 KB +0.07% +17 B 🔺
@sentry/browser (incl. FeedbackAsync) 31.9 KB +0.04% +13 B 🔺
@sentry/react 25.28 KB +0.07% +16 B 🔺
@sentry/react (incl. Tracing) 37.83 KB -0.01% -2 B 🔽
@sentry/vue 26.66 KB +0.02% +5 B 🔺
@sentry/vue (incl. Tracing) 36.68 KB +0.03% +8 B 🔺
@sentry/svelte 22.65 KB +0.06% +12 B 🔺
CDN Bundle 23.76 KB +0.04% +8 B 🔺
CDN Bundle (incl. Tracing) 36.53 KB +0.06% +19 B 🔺
CDN Bundle (incl. Tracing, Replay) 70.86 KB +0.02% +12 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 76.16 KB +0.01% +7 B 🔺
CDN Bundle - uncompressed 69.61 KB -0.11% -74 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 108.28 KB -0.07% -74 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 219.66 KB -0.04% -74 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 232.85 KB -0.04% -74 B 🔽
@sentry/nextjs (client) 37.6 KB +0.04% +12 B 🔺
@sentry/sveltekit (client) 35.44 KB -0.02% -5 B 🔽
@sentry/node 115.71 KB - -
@sentry/node - without tracing 90.1 KB - -
@sentry/aws-serverless 99.51 KB - -

View base workflow run

github-actions[bot] avatar May 24 '24 12:05 github-actions[bot]

this fixes half of the issue in that it provides proper names for the xhr prototype functions. however, this is still a culprit:

Function.prototype.toString = function() {
    if (this.__sentry_original__) {
        return this.__sentry_original__.toString();
    }
    return origFunctionToString.apply(this);
}

this implementation per se is fine, unfortunately it conflicts with Function.prototype.name - which is not wrapped like this. so i guess this would also need something like

const origFunctionName = Function.prototype.name;
Object.defineProperty(Function.prototype, "name", {
  get() {
    if (this.__sentry_original__) {
      return this.__sentry_original__.name;
    }
    return origFunctionName.apply(this);
  }
});

~~this is just a poc, not sure if this code works, but that would bring consistency between Function.prototype.toString and Function.prototype.name i guess~~ edit: just noticed this doesn't work because the name property is not defined on the prototype, but only on the instance. this makes it a bit more difficult to make the behavior consistent...

DerGernTod avatar Aug 26 '24 09:08 DerGernTod

@DerGernTod makes sense. In your original issue, you're describing a symptom rather than the thing you would like to achieve.

I changed the implementation to be based on proxies. I think that should resolve most if not all of your problems. Can you confirm that?

lforst avatar Aug 26 '24 11:08 lforst

i tried your changes with our tests and looks like it works fine. nice, thanks!

DerGernTod avatar Aug 26 '24 12:08 DerGernTod