[console] use user-defined .toString instead of internal one |
||||||||
Issue description
Chrome Version : 56.0.2924.87
URLs (if applicable) : N/A
Other browsers tested:
Safari: OK (latest + Tech Preview Release 24)
Firefox: FAIL
IE: FAIL
What steps will reproduce the problem?
(1) [OPTIONAL] Read up on the issue here: https://github.com/whatwg/console/issues/26
(2) Open DevTools on any page
(3) Run the code from the following jsbin: https://jsbin.com/gepeni/3/edit?js,console.
What is the expected result?
It is expected that a timer's label will computed using `label.toString()` if the label is an object, therefore code inside `label.toString()` will be invoked, and in this case an exception will be thrown, propagated to the user, and no timer will be started.
What happens instead?
You'll see that the `toString()` "method" of an object passed into console.time as a timer label is never called, and therefore its return value or thrown exceptions are never honored. Instead some sort of odd native string conversion is done (something to the effect of `label.__proto__.toString.call(label)`) as our custom `toString` is ignored.
Please provide any additional information below. Attach a screenshot if
possible.
More information can be found in the following whatwg console issue thread: https://github.com/whatwg/console/issues/26
Also affects: console.count(object), console.log(object)
,
Mar 2 2017
Tested in chrome # 56.0.2924.87 and Canary #58.0.3028.0 on win 10.0,Ubuntu 14.04 & Mac 10.12.3.Please find the screen shots for your reference. @ domfarolino: Could you please let me know if i have missed anything and if possible, provide us with a OS details and expected behaviour of the issue which would help us to triage the issue further. Thanks in Advance.
,
Mar 2 2017
Thanks for the screenshot. We are passing in an object as the timer-we-want-to-start's label. The spec indicates that a label whose type an object, should be coerced to a string. This should happen by calling `label.toString()`. For example if you were to take the following code:
--------------
x = {
toString() {
console.log('Hey, we defined our own toString!');
throw new Error('ehhhhh');
}
}
x.toString();
--------------
You'll see an error get thrown in the DevTools because we called x.toString(). When we call console.time(label) we expect `label.toString()` to be called in an attempt to convert the object to a string in the case where label is an object. Normally Object.prototype.toString would get called, however we have defined our own conversion mechanism. Therefore in the jsbin we should see some sort of log indicating our custom `toString` was called, and the resulting thrown error. Instead, Chrome just handles object => string conversion (in this case) on its own, ignoring our custom `toString` "method". You can run the bin code in the Safari console to see the spec-compliant behavior. Does this clear it up a bit? Chrome is basically ignoring our toString method which the discussion in https://github.com/whatwg/console/issues/26 seems to consider sub-optimal.
,
Mar 2 2017
Thank you for providing more feedback. Adding requester "rbasuvula@chromium.org" to the cc list and removing "Needs-Feedback" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 6 2017
Able to reproduce the issue on windows 7, Mac 10.12.2,Linux Ubuntu 14.04 with Chrome stable version-56.0.2924.87 and canary 59.0.3030.0 Manual Bisect: Good Build—52.0.2714.0-Revision (388647) Bad Build— 52.0.2715.0-Revision (388964) Per revision Bisect Tool Info: CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/31fe1233aed27affb741984db8903df127163981..807ec9550e8a31517966636e6a5b506474ab4ea9 Review URL: https://codereview.chromium.org/1859293002 kozyatinskiy@ Kindly take a look and please help us to reassign this issue to a right owner if not with respect to this change. Thanks.!
,
Mar 22 2017
Friendly Ping! Still able to reproduce the issue on Ubuntu 14.04 using latest chrome version 59.0.3048.0. kozyatinskiy@ Could you please look into this issue. Thanks!
,
Jun 5 2017
,
Oct 16 2017
,
Nov 28 2017
,
Nov 28 2017
,
Dec 5 2017
,
Dec 5 2017
,
Dec 20 2017
Hello just pinging this issue to see if there's an update perhaps? We're seeing more issues with `ToString` not being called on other console APIs (%s format specifier for example), and I'm wondering if fixes related to this issue would fix both cases or if I'll need to file a separate issue. I'm assuming I'll need to file a separate issue but I just wanted to check in.
,
Jan 3 2018
This issue is enough and it is more about decision, should we call custom toString or not and should console.log throws if toString throws. Could you share the link with discussion about this? I believe that if console.log starts throw it can "break the web".
,
Jan 11 2018
Largely we've decided that if toString() throws, then console APIs that are supposed to call the throwing-toString should resurface those errors, and by extension throw as well. Conversations around this can be seen here: - https://github.com/whatwg/console/issues/26 - https://github.com/whatwg/console/issues/88 Although the above indicates that when a custom toString() is called (and should be given the current console standard and its interaction with ECMAScript) and throws, the errors should be re-thrown, https://github.com/whatwg/console/issues/113#issuecomment-353150457 and https://github.com/whatwg/console/pull/123 are on-track to now make console.log NOT throw when trying to format-specify Symbols with `%s`, since we're special-casing them. With `%s` specifying an object or other primitives, however, we still expect erroneous custom toString() to rethrow. Thoughts? If we ultimately want to change this, feel free to file an issue on the spec.
,
Jan 12 2018
Note I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=801442 as a more broad issue tackling all of the issues associated with applying format specifiers to Symbols. For example, applying %i/%d/%f specifiers on Symbols should throw, as their associated abstract operations throw, and that error should be resurfaced. The %s specifier however, should not throw given the latest (pending*) spec change. *Awaiting test PR (https://github.com/w3c/web-platform-tests/pull/9008) to be merged. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by nyerramilli@chromium.org
, Feb 28 2017