Skip to content
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: Fix uncaught errors while testing on Cast #7345

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

joeyparrish
Copy link
Member

We get strange uncaught errors sometimes on Cast devices. These are unreadable "script error" events that have nothing to do with our tests (see https://sentry.io/answers/script-error/), and we intend to ignore them.

However, our existing logic to ignore those is not sufficient, because in addition to our own error handler (via window.addEventListener("error")), Jasmine has its own unconditional error handler (via window.onerror).

To take complete control over how these are handled, we need to remove Jasmine's handler.

Jasmine's handler is installed at the top of its execute() function, so our top-level beforeAll() is the best place to remove it.

We get strange uncaught errors sometimes on Cast devices.  These are unreadable "script error" events that have nothing to do with our tests (see https://sentry.io/answers/script-error/), and we intend to ignore them.

However, our existing logic to ignore those is not sufficient, because in addition to our own error handler (via window.addEventListener("error")), Jasmine has its own unconditional error handler (via window.onerror).

To take complete control over how these are handled, we need to remove Jasmine's handler.

Jasmine's handler is installed at the top of its execute() function, so our top-level beforeAll() is the best place to remove it.
@joeyparrish joeyparrish requested a review from avelad September 19, 2024 23:02
@shaka-bot
Copy link
Collaborator

Incremental code coverage: No instrumented code was changed.

@avelad avelad added the component: tests The issue involves our automated tests (generally; otherwise use a more specific component) label Sep 20, 2024
@avelad avelad added this to the v4.12 milestone Sep 20, 2024
@avelad avelad merged commit cc3dc1d into shaka-project:main Sep 20, 2024
16 of 17 checks passed
@joeyparrish joeyparrish deleted the uncaught-script-errors branch September 20, 2024 16:41
avelad pushed a commit that referenced this pull request Sep 24, 2024
We get strange uncaught errors sometimes on Cast devices. These are
unreadable "script error" events that have nothing to do with our tests
(see https://sentry.io/answers/script-error/), and we intend to ignore
them.

However, our existing logic to ignore those is not sufficient, because
in addition to our own error handler (via
window.addEventListener("error")), Jasmine has its own unconditional
error handler (via window.onerror).

To take complete control over how these are handled, we need to remove
Jasmine's handler.

Jasmine's handler is installed at the top of its execute() function, so
our top-level beforeAll() is the best place to remove it.
avelad pushed a commit that referenced this pull request Sep 24, 2024
We get strange uncaught errors sometimes on Cast devices. These are
unreadable "script error" events that have nothing to do with our tests
(see https://sentry.io/answers/script-error/), and we intend to ignore
them.

However, our existing logic to ignore those is not sufficient, because
in addition to our own error handler (via
window.addEventListener("error")), Jasmine has its own unconditional
error handler (via window.onerror).

To take complete control over how these are handled, we need to remove
Jasmine's handler.

Jasmine's handler is installed at the top of its execute() function, so
our top-level beforeAll() is the best place to remove it.
joeyparrish added a commit to joeyparrish/shaka-player that referenced this pull request Sep 26, 2024
In shaka-project#7345, we disabled Jasmine's global error handler.  However,
jasmine still tries to call this.  So instead of replacing it with
null, replace it with a stub.

Also add comments about why we are able to do this and still handle
global errors in our own way.
joeyparrish added a commit that referenced this pull request Sep 26, 2024
In #7345, we disabled Jasmine's global error handler. However, jasmine
still tries to call this. So instead of replacing it with null, replace
it with a stub.

Also add comments about why we are able to do this and still handle
global errors in our own way.
joeyparrish added a commit that referenced this pull request Sep 26, 2024
We get strange uncaught errors sometimes on Cast devices. These are
unreadable "script error" events that have nothing to do with our tests
(see https://sentry.io/answers/script-error/), and we intend to ignore
them.

However, our existing logic to ignore those is not sufficient, because
in addition to our own error handler (via
window.addEventListener("error")), Jasmine has its own unconditional
error handler (via window.onerror).

To take complete control over how these are handled, we need to remove
Jasmine's handler.

Jasmine's handler is installed at the top of its execute() function, so
our top-level beforeAll() is the best place to remove it.
joeyparrish added a commit that referenced this pull request Sep 26, 2024
In #7345, we disabled Jasmine's global error handler. However, jasmine
still tries to call this. So instead of replacing it with null, replace
it with a stub.

Also add comments about why we are able to do this and still handle
global errors in our own way.
joeyparrish added a commit that referenced this pull request Sep 26, 2024
In #7345, we disabled Jasmine's global error handler. However, jasmine
still tries to call this. So instead of replacing it with null, replace
it with a stub.

Also add comments about why we are able to do this and still handle
global errors in our own way.
joeyparrish added a commit that referenced this pull request Sep 26, 2024
In #7345, we disabled Jasmine's global error handler. However, jasmine
still tries to call this. So instead of replacing it with null, replace
it with a stub.

Also add comments about why we are able to do this and still handle
global errors in our own way.
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Nov 19, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Nov 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: tests The issue involves our automated tests (generally; otherwise use a more specific component) status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants