-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for specifying pluggable integrations with Lazy Loader directly from CDN #2476
Comments
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.
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()]
});
});
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! :) |
Thanks @kamilogorek ,
We thought about it, but decided that we'd rather use the official build. Especially since the version discrepancy won't be an issue.
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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Why the following pattern results in skipped
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 |
Appears like Sentry is auto-initing on main bundle 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 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 |
@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. But your solution is current best |
Thank you for the feedback! I have created an issue over here: getsentry/sentry#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: #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?The text was updated successfully, but these errors were encountered: