Closed dryoma closed 1 year ago
Is it also the recommended way to use non-default integrations with the Lazy Loader?
It's fine to use it like this, although try to use the latest version, eg. https://browser.sentry-cdn.com/5.13.2/dedupe.min.js for dedupe.
Won't the discrepancy between the automatically set version in the Loader and the fixed version in integrations scripts be an issue?
Not if you'll stick with the same major version, which can be selected in the project settings. Eg 5.x instead of the latest, but we don't plan to release v6 anytime soon anyway. Right now, the only thing you have to remember is to configure the integration in the provided callback:
Sentry.onLoad(function() {
Sentry.init({
integrations: [new Sentry.Integrations.Dedupe()]
});
});
Maybe there is a way to specify the integrations I want to use as parameters to the Loaders scr as, say, Google Fonts do?
Unfortunately not, as we didn't have such a feature request yet. It'd also take quite a lot of work to make it work correctly.
Also, one thing worth noting is that loader is also open-sourced and you can create your own bundle of Sentry+Dedupe and point the loader to load it instead of CDN version - https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/loader.js
Hope that helps! :) Cheers!
Thanks @kamilogorek ,
you can create your own bundle of Sentry+Dedupe
We thought about it, but decided that we'd rather use the official build. Especially since the version discrepancy won't be an issue.
Unfortunately not, as we didn't have such a feature request yet.
Can this ticket be considered as one? ;)
Done :)
Thanks @kamilogorek
Just stumbled upon another reason why we'd like the ability to bundle non-default Integrations on demand. When loading corresponding scripts for them, the scripts sometimes would fail to load. The same happens for our scripts as well sometimes, my guess is that some network issues are the culprit. But the result is that errors like this keep popping up. The nasty thing about them is that we can't even group them because Sentry fails to initialize our config because of that error, so notifications keep coming. There are options, like checking if the integration has loaded and only then initialize it; but I really don't want to end up with a client that spams thousands of errors without Dedupe keeping it at bay.
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 🥀
@kamilogorek
Why the following pattern results in skipped Sentry.init
config and using a default config instead?
<script src='loader.js' crossorigin="anonymous"></script>
<link rel="preload" href="integration.js" as="script" crossorigin="anonymous" />
<script>
Sentry.onLoad(function() {
const script = document.createElement('script');
script.src = 'integration.js';
script.crossOrigin = 'anonymous';
document.scripts[0].parentNode.insertBefore(script, document.scripts[0]);
script.addEventListener('load', function() {
Sentry.init({
// this config is skipped by Sentry
});
});
</script>
This forces to either include integration script in a render-blocking manner, or loading it via dynamic script creation right away (without relying on its load
) and hoping it will load faster than main Sentry bundle
Appears like Sentry is auto-initing on main bundle load
, but integration load
will be called on the next tick earliest, so already late. If switching Sentry.onLoad
and integration load
there's a chance Sentry bundle will load first, and calling Sentry.onLoad
on integration load
won't have an effect as Sentry load
already happened and it is again already inited.
I guess as a mitigation we could load integration async right after loader and hope it will load before Sentry. But in case it loads after Sentry we could dynamically apply it, so that errors recorded prior the load won't have it applied, but further errors will. But I guess this is also not possible
So with the following config:
<script src="loader here"></script>
<script>
Sentry.onLoad(function() {
Sentry.init({
// your config here
});
});
</script>
This config should take precedence over the default loader config, and should allow to provide further integrations as well! Is that not working for you?
It will only apply the config (and any added integrations) only once an error happens, if you only use the loader for errors. If that is not working for you, that would be a bug! We actually have a test that should hopefully cover this here: https://github.com/getsentry/sentry-javascript/blob/develop/packages/browser-integration-tests/loader-suites/loader/onLoad/customIntegrations/init.js
It is working, it's not working when Sentry.init
is not executed in the same call stack as Sentry.onLoad
(this happens when Sentry.init
waits for load
event of integration script in case it's loaded asynchronously, see my example above).
Current Sentry code works as expected, it's just a problem with loading integrations where we need to load them together with a loader in a render-blocking manner to guarantee they're available by the time Sentry.onLoad
is called (because it's the last opportunity to set init config), instead of loading integrations asynchronously
@andreyvolokitin if you want to use that pattern, I would recommend calling
const client = Sentry.getCurrentHub().getClient();
if (client) {
client.addIntegration(new MyIntegrationHere());
}
This allow you to lazily add integrations after the fact.
Wow, thank you, that would be exactly what I need. I guess specifying the integration list as an option for the loader to then download full Sentry bundle, including specified integrations, from CDN would be somewhat ideal (the actual topic of this issue): e.g. <script src="loader.js" data-integrations="rewriteframes, someintegration"></script>
. This way we would have all required code before init
and also won't block rendering any further
But your solution is current best
Thank you for the feedback!
I have created an issue over here: https://github.com/getsentry/sentry/issues/47540 to track this specifically. I believe it may make sense to do something like this, but we'll need to investigate how well that works or if that has unintended side effects. I'll close this issue for now, and we can track progress on this particular topic over in the new issue - thank you!
Is there a way to lazy load Sentry v8 SDK?
At the moment, if you look at my website, 30% of bundle size is just Sentry.
https://pillser.com/vitamins/vitamin-a
That's a pretty steep cost to pay just for having error reporting.
I tried removing it and it gets me to 100 Lighthouse score.
Loader for v8 being tracked here: https://github.com/getsentry/sentry-javascript/issues/12187
Loader for v8 being tracked here: #12187
I am not using loader though.
I am importing it like this:
import * as Sentry from '@sentry/remix';
Sentry.init({
dsn: SENTRY_DSN,
environment: ENVIRONMENT,
normalizeDepth: 7,
release: RELEASE_VERSION,
replaysOnErrorSampleRate: 0,
replaysSessionSampleRate: 0,
tracePropagationTargets: ['localhost', /^https:\/\/pillser\.com/u],
tracesSampleRate: 0,
tunnel: new URL('/sentry', APP_URL).href,
});
And it is flagged by Lightship like so:
If you want to only use errors, the recommended way to do this and get lazy loading is to use the loader script. This will only inject the Sentry SDK once an error actually happens. See https://docs.sentry.io/platforms/javascript/install/loader/ for details.
If you want to use the NPM package, there is currently no way to get lazy loading out of the box - you'll always have to include the base package at least. Additional features like performance or replay can be lazy loaded, if wanted.
I know that in order to use pluggable integrations with the CDN version a corresponding script needs to be included on a page:
Is it also the recommended way to use non-default integrations with the Lazy Loader? Won't the discrepancy between the automatically set version in the Loader and the fixed version in integrations scripts be an issue? Maybe there is a way to specify the integrations I want to use as parameters to the Loaders scr as, say, Google Fonts do? Or maybe something like
https://browser.sentry-cdn.com/5.x/dedupe.min.js
would be possible so that the version of an integration could correspond to the verison that the Loader fetches?