-
-
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
test(loader): Update Loader Script & test error in sentryOnLoad
#13952
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change! Left a comment in getsentry/sentry#78993 (comment) but apart from that I think this is good to go.
async ({ getLocalTestUrl, page }) => { | ||
const url = await getLocalTestUrl({ testDir: __dirname }); | ||
const req = await waitForErrorRequestOnUrl(page, url); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: we could assert on the console output here so that we ensure that the error is logged, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated this to assert on the error message!
If a user has an error in their `sentryOnLoad` function for the Loader Script, we do try-catch it today, but we stop any further processing, leading to possible issues down the line (e.g. errors not being sent to Sentry etc). This PR changes this so that we catch errors in this first, and continue if it happens. This means that we'll still do the default `Sentry.init()` and send the error that triggered lazy loading to Sentry - this was previously swallowed by the catch. Closes getsentry/sentry-javascript#13939 Test added here: getsentry/sentry-javascript#13952
…3952) Updates the loader & adds tests for getsentry/sentry#78993
If a user has an error in their `sentryOnLoad` function for the Loader Script, we do try-catch it today, but we stop any further processing, leading to possible issues down the line (e.g. errors not being sent to Sentry etc). This PR changes this so that we catch errors in this first, and continue if it happens. This means that we'll still do the default `Sentry.init()` and send the error that triggered lazy loading to Sentry - this was previously swallowed by the catch. Closes getsentry/sentry-javascript#13939 Test added here: getsentry/sentry-javascript#13952
Updates the loader & adds tests for getsentry/sentry#78993