Skip to content

/ servo

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

Implement error reporting for workers. #13193

Merged
merged 3 commits into from Sep 9, 2016
Merged

Implement error reporting for workers. #13193

merged 3 commits into from Sep 9, 2016
+169 −128

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 7, 2016 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Sep 7, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/bindings/global.rs, components/script/dom/webidls/WorkerGlobalScope.webidl, components/script/dom/workerglobalscope.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/script/dom/worker.rs
@nox
Copy link
Member

nox commented Sep 7, 2016

Two questions and maybe just as many follow-up issues, otherwise r=me.

-S-awaiting-review +S-awaiting-answer

Reviewed 22 of 22 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

components/script/dom/dedicatedworkerglobalscope.rs, line 360 [r1] (raw file):

        self.in_error_reporting_mode.set(true);

        // Steps 3-12.

Put a TODO for step 6? You don't seem to support it.

components/script/dom/dedicatedworkerglobalscope.rs, line 372 [r1] (raw file):

        // Step 13.
        let handled = !event.upcast::<Event>().fire(self.upcast::<EventTarget>());

Do you think we should change the return type of Event::fire to a more intelligible enum? The double negation here hurts my brain.

Comments from Reviewable
@nox nox assigned nox and unassigned SimonSapin Sep 7, 2016
@Ms2ger Ms2ger force-pushed the error-workers branch from a1dea38 to cebf2d6 Sep 7, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Sep 7, 2016

Filed #13195 and #13196.

@bors-servo r=nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 7, 2016

📌 Commit cebf2d6 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 7, 2016

⌛ Testing commit cebf2d6 with merge d79b63c...
bors-servo added a commit that referenced this pull request Sep 7, 2016
@bors-servo
Auto merge of #13193 - servo:error-workers, r=nox
Loading status checks…
d79b63c 
Implement error reporting for workers.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13193)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 7, 2016

💔 Test failed - mac-rel-wpt
@highfive
Copy link

highfive commented Sep 7, 2016 

  ▶ TIMEOUT [expected PASS] /_mozilla/css/iframe/hide_layers2.html

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/digest/digest.worker
  └   → Error in worker http://web-platform.test:8000/resources/testharness.js: tests.promise_tests is undefined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/encrypt_decrypt/aes_cbc.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/encrypt_decrypt/aes.js: subtle is undefined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/encrypt_decrypt/aes_ctr.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/encrypt_decrypt/aes.js: subtle is undefined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/encrypt_decrypt/rsa.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/encrypt_decrypt/rsa.js: subtle is undefined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/encrypt_decrypt/aes_gcm.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/encrypt_decrypt/aes.js: subtle is undefined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_AES-CBC.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_AES-CTR.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_AES-GCM.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_AES-KW.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_ECDH.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_ECDSA.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_HMAC.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_RSA-OAEP.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_RSA-PSS.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/failures_RSASSA-PKCS1-v1_5.</span><span class="stdout">worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/failures.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_AES-CBC.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_AES-CTR.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_AES-GCM.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_AES-KW.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_ECDH.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_ECDSA.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_HMAC.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_RSA-OAEP.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_RSA-PSS.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /WebCryptoAPI/generateKey/successes_RSASSA-PKCS1-v1_5.worker
  └   → Error in worker http://web-platform.test:8000/WebCryptoAPI/generateKey/successes.js: CryptoKey is not defined

  ▶ ERROR [expected TIMEOUT] /workers/semantics/multiple-workers/005.html
  └   → SharedWorker is not defined
@Ms2ger Ms2ger force-pushed the error-workers branch from cebf2d6 to 9b8bbe9 Sep 7, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Sep 7, 2016

@bors-servo r=nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 7, 2016

📌 Commit 9b8bbe9 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 7, 2016

⌛ Testing commit 9b8bbe9 with merge 8f3e54b...
bors-servo added a commit that referenced this pull request Sep 7, 2016
@bors-servo
Auto merge of #13193 - servo:error-workers, r=nox
Loading status checks…
8f3e54b 
Implement error reporting for workers.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13193)
<!-- Reviewable:end -->
@highfive
Copy link

highfive commented Sep 7, 2016 

  ▶ TIMEOUT [expected OK] /webgl/conformance-1.0.3/conformance/textures/origin-clean-conformance.html

  ▶ ERROR [expected OK] /workers/WorkerGlobalScope_ErrorEvent_lineno.htm
  └   → Error Message
@Ms2ger Ms2ger force-pushed the error-workers branch from 8deab3b to 22270f4 Sep 8, 2016
@highfive highfive removed the S-tests-failed label Sep 8, 2016
@nox
Copy link
Member

nox commented Sep 8, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Sep 8, 2016

📌 Commit 22270f4 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

⌛ Testing commit 22270f4 with merge d4206fc...
bors-servo added a commit that referenced this pull request Sep 9, 2016
@bors-servo
Auto merge of #13193 - servo:error-workers, r=nox
Loading status checks…
d4206fc 
Implement error reporting for workers.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13193)
<!-- Reviewable:end -->
@wafflespeanut wafflespeanut mentioned this pull request Sep 9, 2016
Merged
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

⛄ The build was interrupted to prioritize another pull request.
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

☔ The latest upstream changes (presumably #13205) made this pull request unmergeable. Please resolve the merge conflicts.
Ms2ger added 3 commits Sep 8, 2016
@Ms2ger
Allow uncaught exceptions in worker runtime error tests.
59cf735
@Ms2ger
Implement error reporting for workers.
60142f8 
Fixes #3311.
Fixes #13158.
@Ms2ger
Note the lacking support for muted errors.
Loading status checks…
3978359
@Ms2ger Ms2ger force-pushed the error-workers branch from 22270f4 to 3978359 Sep 9, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Sep 9, 2016

@bors-servo r=nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

📌 Commit 3978359 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

⌛ Testing commit 3978359 with merge 1d40075...
bors-servo added a commit that referenced this pull request Sep 9, 2016
@bors-servo
Auto merge of #13193 - servo:error-workers, r=nox
Loading status checks…
1d40075 
Implement error reporting for workers.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13193)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev
@bors-servo bors-servo merged commit 3978359 into master Sep 9, 2016
3 of 4 checks passed
3 of 4 checks passed
dependency-ci Failed dependency checks
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the error-workers branch Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Ms2ger @highfive @nox @bors-servo @SimonSapin
You can’t perform that action at this time.