-
-
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
Improve error throwing behaviour in browser integration test #11678
Comments
@Lms24, I think the 3rd one would be the most intuitive if it works for every case we test (and the cases we don't properly test due to the current limitations). I can give it a shot if that makes sense? |
Yup, sounds good to me! |
@onurtemizkan feel free to convert the tests added from #11666 after we merge that in to test everything out! |
AbhiPrasad
added a commit
that referenced
this issue
Apr 24, 2024
ref #11084 This test ports `packages/browser/test/integration/suites/onunhandledrejection.js` playwright. Because of the same limitations as outlined with the on error tests #11666, I had to use calls to `window.onunhandledrejection` to simulate these tests instead of just using `Promise.reject` to test the handler. #11678 tracks being able to fix this so we can avoid directly calling `window.onunhandledrejection` to test. As `onunhandledrejection.js` was the last suite to use the old integration tests, I fully removed that code and the corresponding GH action workflow. I also removed the monorepo deps on `karma`, `chai` and `sinon`. Extremely satisfying.
AbhiPrasad
pushed a commit
that referenced
this issue
Apr 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem Statement
Currently, it's quite annoying to just
throw
an error in a browser integration test and to check for the resulting payload. The intuitive way would be to just top-level callthrow new Error()
insubject.js
. However, the resulting error gets thrown as a "Script Error" because it's thrown synchronously when the subject bundle JS is evaluated by the browser. This error is filtered by the SDK by default for good reason.Solution Brainstorm
Ideally, we can work around this in one way or another so that we can have a simple way of throwing errors in a test that actually make it through the SDK. I have a couple of ideas but we'd need to check what the best approach is. Also I'm definitely open to other ideas.
subject.js
code in asetTimeout
call. This will cause the initial bundle evaluation to succeed and potential errors should be thrown as intended by testers.async-subject.js
) that does what's suggested in 1.subject.js
would stay sync as previously.triggerError(err: unkown)
) that callspage.evaluate
. This should work 🤔 but admittedly I didn't test it yetThe text was updated successfully, but these errors were encountered: