stayallive / wp-sentry

A (unofficial) WordPress plugin reporting PHP and JavaScript errors to Sentry.
https://wordpress.org/plugins/wp-sentry-integration/
MIT License
305 stars 48 forks source link

Version bump and lazyloading option #119

Closed midweste closed 2 years ago

midweste commented 2 years ago

May need to do a bit more cleanup here. Working on fixing how the "Send JavaScript test event" works on the test page.

Per lazyloading instructions:

Finally, if you want to control the timing yourself, you can call Sentry.forceLoad(). You can do this as early as immediately after the loader runs (which has the same effect as setting data-lazy="no") and as late as the first unhandled error, unhandled promise rejection, or call to Sentry.captureMessage or Sentry.captureEvent (which has the same effect as not calling it at all). Note that you can't delay loading past one of the aforementioned triggering events.

https://docs.sentry.io/platforms/javascript/install/lazy-load-sentry/#:~:text=Finally%2C%20if%20you,aforementioned%20triggering%20events.

Which if I'm reading it correctly means that we wont get an eventId from the test call to Sentry.captureMessage as it needs an unhandled error or promise rejection before it loads the full bundle and then it will submit all the captureMessages and exceptions

var eventId = Sentry.captureMessage('This is a test message sent from the Sentry WP JavaScript integration.');

I moved the onLoad function from the sdk files into its own file, I think this should be ok.

stayallive commented 2 years ago

It's going to take me some time to look this over, this requires more changes than I figured and I'm a little busy at the moment. Thanks for the work and info already.

midweste commented 2 years ago

No worries, let me know if you need anything. Thank you!

stayallive commented 2 years ago

Hey @midweste I'm very sorry to keep this so long but I've decided against merging this.

I do not feel comfortable in doing this and adding a second script to load also.

If you need this kind of optimisation doing it "manually" seems like a better choice.

I might revisit this in the future if we can find a cleaner solution to this that does not require adding a second script to load when you are not using this feature. Sentry team is also doing massive work in optimising the browser SDK so at some point hopefully this will not be needed anyway.

midweste commented 2 years ago

I'll take a look and see if I can find a better way that fits with your project goals and eliminates any extra requests. Thanks for responding and your consideration