New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 31 users

Issue metadata

Status: Fixed
Owner:
OOO until 4th
Closed: Aug 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

Add an 'error' parameter to 'window.onerror' handlers, and the ErrorEvent interface.

Reported by bj...@leftronic.com, Sep 7 2012

Issue description

This is not actually a defect. It is a feature request, but there is no feature template available to choose from.

Me and many engineers would like the non-standard call stack available on errors in a catch block to also be available in onerror. This would help debug client errors out in the wild. It is becoming common for engineering teams to post client errors back to the server for logging and alert purposes. Having a call stack available in addition to the error message and a line and a url would be very helpful. Check out this discussion on mozilla:

https://bugzilla.mozilla.org/show_bug.cgi?id=355430

Also there are lots of discussions online about wanting such a feature.
 
Labels: -OS-Mac -Area-Undefined -Type-Bug OS-All Area-WebKit Type-Feature Feature-DevTools
Status: Untriaged

Comment 2 by vsevik@chromium.org, Oct 12 2012

Owner: yu...@chromium.org
Status: Assigned

Comment 3 by yu...@chromium.org, Oct 12 2012

Cc: yu...@chromium.org
Labels: -Feature-DevTools
Owner: ----
Status: Available
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-WebKit Cr-Content
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 5 2013

Labels: -Cr-Content Cr-Blink

Comment 6 by ytp...@gmail.com, May 2 2013

See also WebKit bug on the same subject: https://bugs.webkit.org/show_bug.cgi?id=55092

Comment 7 by ytp...@gmail.com, May 2 2013

It looks like there's already infrastructure in place to sanitize the info passed to onerror when cross-domain requires it (see  issue #159566  for example), so it seems like this should be feasible without raising any security issues.
Labels: Hotlist-GoogleApps
Any updates on this?
Labels: Cr-Platform-DevTools-JavaScript

Comment 10 by yu...@chromium.org, Jul 17 2013

@komoroske: why DevTools? window.onerror is a web facing API that doesn't depend on Chromium DevTools.
Cc: arv@chromium.org
Ah, I see. I didn't think about it too carefully; I was just running through a long list of issues to triage.

What do you think is a better home for this? Who would be the person or team who would actually decide to implement this? 

+arv in case he has ideas (or is the right person himself).

Comment 12 by yu...@chromium.org, Jul 17 2013

Labels: -Cr-Platform-DevTools-JavaScript Cr-Blink-DOM Cr-Blink-Bindings
It's hard to say. I had a few spare cycles and supported onerror handler currently specified in HTML5 spec. Not sure who would be the right person/team to add the stack traces. I think DOM or Bindings is a more appropriate component for this feature.

Comment 13 by mkwst@chromium.org, Jul 17 2013

Cc: abarth@chromium.org mkwst@chromium.org
I'm interested in poking at this. I might even have some time to do so.

If we expose the same stack (with the same limitations) as we expose on the thrown error, I don't think we'd be opening any new opportunities for leakage. That said, +abarth who certainly has opinions about exposing detail like this to JavaScript.

Comment 14 Deleted

Comment 15 by mkwst@chromium.org, Jul 17 2013

Cc: esprehn@chromium.org
I did a bit more digging. Mozilla seems interested in the feature[1], Apple has explicitly rejected it[2] (though there's still interest from Adobe and other in other WebKit bugs, like [3]). Before doing this, we'd probably want to a) poke blink-dev@ with an intent to implement to chat about the feature, b) poke WHATWG (to follow up on threads/concerns like [4] and [5]).

+esprehn, since he was on those threads. :)

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=355430
[2]: https://bugs.webkit.org/show_bug.cgi?id=104408
[3]: https://bugs.webkit.org/show_bug.cgi?id=55092
[4]: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-November/038156.html
[5]: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2013-July/040019.html
I'm sure there's a way to do this securely.  We just need to be careful.

Comment 17 by mkwst@chromium.org, Jul 25 2013

Owner: mkwst@chromium.org
Status: Assigned
Hixie has added the exception object itself as the 5th parameter, which would include the stack. That takes care of b) above, I suppose. :)

I'll follow up on a) today.

Comment 18 by mkwst@chromium.org, Jul 25 2013

http://html5.org/tools/web-apps-tracker?from=8085&to=8086 is the change in question.

Comment 19 by mkwst@chromium.org, Jul 25 2013

Summary: Add an 'error' parameter to 'window.onerror' handlers, and the ErrorEvent interface. (was: Please add a call stack to onerror to help developers debug issues users are experiencing)

Comment 20 by mkwst@chromium.org, Jul 27 2013

 Issue 256365  has been merged into this issue.
Project Member

Comment 21 by bugdroid1@chromium.org, Aug 2 2013

The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=155454

------------------------------------------------------------------------
r155454 | mkwst@chromium.org | 2013-08-02T21:51:12.899465Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/constructors/error-event-constructor.html?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ErrorEvent.idl?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/v8/V8Initializer.cpp?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/deprecated_code_generator_v8.pm?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/v8/WorkerScriptController.cpp?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/workers/WorkerMessagingProxy.cpp?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/v8/V8ErrorHandler.cpp?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/IDLAttributes.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/constructors/error-event-constructor-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ScriptExecutionContext.cpp?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-01-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-10-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ErrorEvent.h?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-02-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-11-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-isolatedworld-02.html?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-12-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-03-expected.txt?r1=155454&r2=155453&pathrev=155454
   A http://src.chromium.org/viewvc/blink/trunk/Source/bindings/v8/custom/V8ErrorEventCustom.cpp?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/resources/onerror-test.js?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-04-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ScriptExecutionContext.h?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-05-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-06-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-09-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/v8/V8HiddenPropertyName.h?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-isolatedworld-01-expected.txt?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/bindings.gypi?r1=155454&r2=155453&pathrev=155454
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/window-onerror-isolatedworld-02-expected.txt?r1=155454&r2=155453&pathrev=155454

Add 'error' parameter to 'window.onerror' and 'ErrorEvent'.

The HTML5 spec recently added the error object to both the
'window.onerror' callback and the 'ErrorEvent' interface in order to
improve developers' ability to debug issues that cause unexpected
exceptions in the wild[1]. This patch brings Blink's behavior into line
with that update.

Currently, we generate an ErrorEvent object after doing sanitization.
This patch refactors that code such that we generate an ErrorEvent when
V8 calls back into the bindings (V8Initializer::messageHandlerInWorker
and V8Initializer::messageHandlerInMainThread). Before we hand that
event off to core for sanitization and dispatch, we grab the wrapper
for the world in which the exception was thrown, and set the exception
object as a hidden value (V8HiddenPropertyName::error()).

On the other end, we grab that exception object in the custom 'error'
getter of 'V8ErrorEvent', and when triggering the 'onerror' handler
via 'V8ErrorHandler::callListenerFunction'. If the value exists, we're
still in the same isolated world, so we can safely return the exception
without leakage. If the value is empty, we've crossed worlds, and
return 'null' instead.

BUG= 147127 

Review URL: https://chromiumcodereview.appspot.com/20351002
------------------------------------------------------------------------
Status: Fixed
Nice work! Thank you for adding this. This is going into Canary soon? 
This is in Canary now. Please bang on it and let us know what breaks. :)
This is great!  Is there a discussion around when to land this into chrome stable?
I always seem to get null no matter what. I never see the error or the stacktrace in canary. I am trying to console log it. I have :

window.onerror = function () {
  console.log(arguments);
}

setTimeout(function () {
  window.scream();
}, 5000);

I get a null for the fifth argument.

arguments ["Script error.", "", 0, 0, null] logging.js:81
Uncaught TypeError: Object [object global] has no method 'scream' 

I looked at the tests for this change and I am not sure how to get a stack trace.
Ah looks like it was because I was loading the script from a separate domain.S ee it now!
This is awesome - the errorevent.error property works great on canary - it makes highlighting the error line possible in this example:

http://david.pencilcode.net/edit/testing/onerror

I'm also wondering when this will be available in chrome stable.

Sign in to add a comment