Let the SharedWorker constructor queue a task to fire an error when parse error occurs #5347
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
elkurin
changed the title
domenic
reviewed
Mar 11, 2020
There was a problem hiding this comment.
I provided some editorial review. It'd be good to get a second check from @annevk and/or @asutherland.
I guess the spec could be clearer on the difference between parse error and error to rethrow. (I wrote up those sections but have forgotten...) I believe the difference is that parse error is only for the immediate script, whereas with modules, the error to rethrow could be a parse error from deeper in the graph. Right? In which case "error to rethrow" is correct, but we should be sure to include some deeper-in-the-graph cases.
|
Thanks for clarifying the difference. Yes, in this spec we would like to catch parse error invoked from both top-level scripts and deeper-in-the-graph scripts. |
domenic
force-pushed
the
parse-error-in-shared-worker
branch
from
a4f1625 to
a6e1445
Compare
domenic
added a commit
that referenced
this pull request
Mar 12, 2020
In #5347 (review) I realized that I had forgotten what the difference was between "parse error" and "error to rethrow", despite being the original author of the spec text. This adds notes explaining the function of each of them, and un-exports "parse error" since it should only be used internally.
domenic
approved these changes
Mar 12, 2020
There was a problem hiding this comment.
This LGTM. If @annevk or @asutherland has time to double-check in the next few days, I'd appreciate it, otherwise I'll land this on Monday or so.
(I will also avoid landing it if the tests PR still has ongoing discussions, which currently seems to be the case.)
domenic
added a commit
that referenced
this pull request
Mar 12, 2020
In #5347 (review) I realized that I had forgotten what the difference was between "parse error" and "error to rethrow", despite being the original author of the spec text. This adds notes explaining the function of each of them, and un-exports "parse error" since it should only be used internally.
|
This seems fine to me, but please don't close #5323 or ensure a follow-up is filed for the remaining work. |
domenic
force-pushed
the
parse-error-in-shared-worker
branch
from
a6e1445 to
ada788c
Compare
domenic
added a commit
that referenced
this pull request
Mar 13, 2020
In #5347 (review) I realized that I had forgotten what the difference was between "parse error" and "error to rethrow", despite being the original author of the spec text. This adds notes explaining the function of each of them, and un-exports "parse error" since it should only be used internally.
domenic
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Mar 23, 2020
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
Mar 31, 2020
…redWorkers, a=testonly Automatic update from web-platform-tests Add WPTs for parse error handling in SharedWorkers Follows whatwg/html#5347. -- wpt-commits: bd6c94ee488cb2c968221a547604fb6c4807a2ca wpt-pr: 22185
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Mar 31, 2020
…redWorkers, a=testonly Automatic update from web-platform-tests Add WPTs for parse error handling in SharedWorkers Follows whatwg/html#5347. -- wpt-commits: bd6c94ee488cb2c968221a547604fb6c4807a2ca wpt-pr: 22185
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Apr 1, 2020
…redWorkers, a=testonly Automatic update from web-platform-tests Add WPTs for parse error handling in SharedWorkers Follows whatwg/html#5347. -- wpt-commits: bd6c94ee488cb2c968221a547604fb6c4807a2ca wpt-pr: 22185 UltraBlame original commit: 4dc14a36b48c3c255bb709ac57eed5a85f3bb67b
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Apr 1, 2020
…redWorkers, a=testonly Automatic update from web-platform-tests Add WPTs for parse error handling in SharedWorkers Follows whatwg/html#5347. -- wpt-commits: bd6c94ee488cb2c968221a547604fb6c4807a2ca wpt-pr: 22185 UltraBlame original commit: 4dc14a36b48c3c255bb709ac57eed5a85f3bb67b
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Apr 1, 2020
…redWorkers, a=testonly Automatic update from web-platform-tests Add WPTs for parse error handling in SharedWorkers Follows whatwg/html#5347. -- wpt-commits: bd6c94ee488cb2c968221a547604fb6c4807a2ca wpt-pr: 22185 UltraBlame original commit: 4dc14a36b48c3c255bb709ac57eed5a85f3bb67b
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.
This PR corresponds to #5323 .
This pull request changes the behavior when shared worker's script invokes parse error.
When a parse error event occurs during fetch, the shared worker used to ignore the error event while it catches and fire an event named error for fetch failure.
We would like to align the behavior of parse error handling with that of fetch failure.
See the doc for more information.
💥 Error: write EPROTO 140049978214272:error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version:../deps/openssl/openssl/ssl/s23_clnt.c:772:
💥 ###
PR Preview failed to build. (Last tried on Mar 13, 2020, 2:25 PM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.