Improve reporting of harness errors in ShadowRealm tests #49387
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
ptomato
commented
Nov 27, 2024
- Better error reporting when setup code fails or an exception is thrown outside a test
- ShadowRealm-in-Worker/AudioWorklet tests fail immediately instead of timing out in that case
This was referenced Nov 27, 2024
Ms2ger
approved these changes
Nov 27, 2024
In shadowrealm-in-window and shadowrealm-in-shadowrealm contexts, if the setup code fails, or an exception is thrown outside of a test, make sure that is detected as a harness error. Previously the test report would just show the unhelpful error message "Unhandled rejection". Now it will show the stringified error message from the ShadowRealm (the actual exception object cannot pass the callable boundary.) Use the trick of passing a throwing callback to setup() to achieve this. I don't think there's any other way to do this outside of the testharness.js file.
…fail Previously, in the shadowrealm-in-dedicatedworker, shadowrealm-in-sharedworker, shadowrealm-in-serviceworker, and shadowrealm-in-audioworklet contexts, if the setup code failed or an exception was thrown outside of a test, the harness would time out because the message signalling completion to the RemoteContext would never be posted. This is wasteful. Instead, catch exceptions during the setup code or importing the test file, and post a synthetic completion message to the message port. We don't have access to the scope in which TestsStatus is defined, so we make a fake status object.
This uses the non-standard `stack` property of Error, if it is present, to provide stack trace information to the caller of shadowRealmEvalAsync(). The stack trace isn't available in shadowrealm-in-serviceworker and shadowrealm-in-audioworklet contexts, because they implement a fake dynamic import with ShadowRealm.prototype.evaluate(), which will wrap any errors from the imported module in a TypeError and clobber any stack info. We could do it by adding a try/catch around the module text, but that would mess with line and column numbers; for now, we'll leave it.
dd39d3e to
21bc331
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.