New issue
Advanced search Search tips

Issue 685303 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

window's onerror is not triggered for exceptions in workers

Reported by bzbar...@mit.edu, Jan 25 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0

Steps to reproduce the problem:
1. Load attached testcase.

What is the expected behavior?
Get an alert from the onerror handler.

What went wrong?
No alert.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 57.0.2987.8 (Official Build) dev (64-bit)  Channel: dev
OS Version: OS X 10.12
Flash Version: 

Safari, Firefox, and Edge all get this right.

See spec at https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2 step 2 of the "not handled" steps for dedicated workers.
 
bar.html
213 bytes View Download
Labels: Needs-Triage-M57

Comment 2 by falken@chromium.org, Jan 26 2017

Cc: nhiroki@chromium.org
Labels: -Pri-2 -OS-Mac OS-All Pri-3
Status: Available (was: Unconfirmed)
See also Issue 590219 and  Issue 640724 . Workers and onerror seems broken.
Labels: -Needs-Triage-M57 M-58
Owner: kozyatinskiy@chromium.org
Status: Assigned (was: Available)
Able to reproduce the issue on Mac 10.12.3, Win 10 and Ubuntu 14.04 using stable 56.0.2924.87 and canary 58.0.3012.0.

Bisect info:
Good: 41.0.2251.0
Bad :41.0.2254.0

NOTE: There is a Blink roll in the range, you might also want to do a Blink bisect.
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/e9eab9d186a06b2cfc235d50458decadb7b33e8b..88d530dfaf92d2f019965b194483370da1a090c6

BLINK CHANGELOG URL:
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?url=/trunk&range=187393%3A187381
Blink CL from Omhaproxy UI :
https://chromium.googlesource.com/chromium/blink/+log/bba3b4794a04380f050929d2942d62101580796e..a0f9118a1a8e6c9214ec7c88c9c2bafc73721837?pretty=fuller&n=10000
Possible suspect from above Blink CL:
Review URL: https://codereview.chromium.org/743153002
kozyatinskiy@: Could you please take a look into this if its related to your change.
Labels: hasbisect
Owner: kozy@chromium.org

Comment 6 by kozy@chromium.org, Jan 3 2018

Cc: -nhiroki@chromium.org kozy@chromium.org
Owner: nhiroki@chromium.org
I believe that this issue is not related to Chrome DevTools and the root is somewhere in reporting exception from worker thread to blink.

Comment 7 by kozy@chromium.org, Jan 3 2018

Cc: -kozy@chromium.org nhiroki@chromium.org
Owner: kozy@chromium.org
Oh, it looks related to my changes, I will take a look.

Comment 8 by kozy@chromium.org, Jan 3 2018

Cc: -nhiroki@chromium.org kozy@chromium.org
Owner: nhiroki@chromium.org
I double checked and I am not sure that my CL can be related.

It looks like we have code that should handle mentioned part of spec [1], this code actually routes unhandled error back to worker global scope. So user can define worker.onerror handler or onerror handler inside worker global scope to catch exception and it looks correct to me.

Forwarded issue to @nhiroki for confirmation.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxy.cpp?rcl=16b3d20c33f49cc89d8f0b0c62cf93fde4675407&l=166

Comment 9 Deleted

Comment 10 by bzbar...@mit.edu, Jan 4 2018

Also, comment 9 is obviously from the wrong account.  Unfortunately, I don't see a way to tell this bug system to stop stealing my logins from other Google things or t edit the commenter info or delete the comment.  :(
c#10: I can delete comment 9 if you'd like, and additionally repost it in a new comment (I'm not sure why I have delete permissions and you don't, though).
comment 9 was from bzbarsky:
"The way errors in workers should work is that an error event fires on the worker global scope.  If that event doesn't get its default action prevented, that default action is to fire an error event on the Worker itself (in the thread where the Worker lives).  If the default action of _that_ is not prevented, that default action is to fire an error event at the global the Worker was created in (which is a Window or another WorkerGlobalScope).

This last step is what was missing in Chrome, if I recall correctly; it's been a while since I reported this issue."
Cc: nhiroki@chromium.org
Labels: WorkerBacklog
Owner: ----
Status: Available (was: Assigned)
Owner: kdillon@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2f6756aea51a388ad85e5c50ec63c778b54733a

commit d2f6756aea51a388ad85e5c50ec63c778b54733a
Author: Katie Dillon <kdillon@chromium.org>
Date: Tue Nov 06 22:36:31 2018

Window onerror is not triggered bug fix

As pointed out in the bug, unhandled errors on worker threads were not
properly being propagated for the parent thread to handle as the spec
requires (https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2).
This fix adds in the code to do that.

Bug:  685303 
Change-Id: Ie3e7ec03153a43efdc6e70b874d6b1a1d8ac35ef
Reviewed-on: https://chromium-review.googlesource.com/c/1287208
Commit-Queue: Katie Dillon <kdillon@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605857}
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/WebKit/LayoutTests/external/wpt/workers/modules/dedicated-worker-import-failure.html
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/WebKit/LayoutTests/external/wpt/workers/modules/resources/new-worker-window.html
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/WebKit/LayoutTests/fast/workers/worker-onerror-04-expected.txt
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/WebKit/LayoutTests/fast/workers/worker-terminate.html
[modify] https://crrev.com/d2f6756aea51a388ad85e5c50ec63c778b54733a/third_party/blink/renderer/core/workers/dedicated_worker_messaging_proxy.cc

Status: Fixed (was: Started)
Labels: -M-58 M-72 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Nice bug fix! chromestatus at: https://www.chromestatus.com/feature/6220798104698880
Labels: -OS-All
Project Member

Comment 20 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bcc307a6074152b3ce6010bb1058b3caa788f4de

commit bcc307a6074152b3ce6010bb1058b3caa788f4de
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Nov 09 07:09:56 2018

Update TestExpectations for virtual/off-main-thread-worker-script-fetch

Already fixed by https://chromium-review.googlesource.com/1287208.

Bug: 655458,  685303 , 835717
Change-Id: Id6df7f593db95846375c77918ede9ffcff6316e6
Reviewed-on: https://chromium-review.googlesource.com/c/1328705
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606755}
[modify] https://crrev.com/bcc307a6074152b3ce6010bb1058b3caa788f4de/third_party/WebKit/LayoutTests/TestExpectations

Sign in to add a comment