Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upInvestigate failure of /html/semantics/scripting-1/the-script-element/module/error-type-1.html after module script landed #25436
Assignees
Labels
Comments
|
As far as I can tell the issue here is that we're triggering an onload event for a script that fails to load because of a broken dependency. |
|
This might be due to how we use Promise.all to signal loads, even when there's a network failure? |
bors-servo
added a commit
that referenced
this issue
Jan 19, 2020
Return the highest priority error from the descendant instead of return the very first one The test failed because we didn't return the highest priority error (which is network error in this case). As Manish mentioned in #25436 (comment), that's because we're using the Promise.all trick to signal loads. If we can avoid relying on Promise.all, maybe we don't need to do a complex logic like this; instead, ideally, we should always finish the module load immediately when we hit network failure so that we don't even need to do the `max()` comparison. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #25436 - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo
added a commit
that referenced
this issue
Jan 20, 2020
Return the highest priority error from the descendant instead of return the very first one The test failed because we didn't return the highest priority error (which is network error in this case). As Manish mentioned in #25436 (comment), that's because we're using the Promise.all trick to signal loads. If we can avoid relying on Promise.all, maybe we don't need to do a complex logic like this; instead, ideally, we should always finish the module load immediately when we hit network failure so that we don't even need to do the `max()` comparison. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #25436 - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Before the IndexSet fix, we will get this failure intermittently. But, I can reproduce this failure consistently.
For merging #23545 first, marked it as FAIL in that PR and fix it separately.
Originally posted by @Manishearth in #23545 (comment)