sentry-javascript
sentry-javascript copied to clipboard
fix(browser): Don't stomp xhr function names
Fixes https://github.com/getsentry/sentry-javascript/issues/12208
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 | - | - |
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 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?
i tried your changes with our tests and looks like it works fine. nice, thanks!