Validate IDL against implementations

[domenic]

For the any... methods, I assume they are probably valid. But for the ones that include type conversions, we need to check. For example:

• In Firefox, console.time({ toString() { throw new Error("boo"); } }) does not throw; in Chrome, it does. Chrome at least seems to follow time(DOMString label).
• On the other hand, console.count({ toString() { throw new Error("boo"); } }) does not throw in Chrome. So count(optional DOMString label = "") doesn't seem quite right.
• In Chrome, console.table({}, (function*() { yield "a"; throw new Error("b"); }())) doesn't seem to throw. So table(any tabularData, optional sequence<DOMString> properties) doesn't seem right. This is related to console.table algorithm #21.

[domfarolino]

time

@domenic Interested in this.

Regarding time & timeEnd it seems that Chrome's behavior has changed, as the first example no longer throws. No idea when this change happened. Firefox still does not throw, and Safari does.

I thought it was pretty weird that in Chrome and Firefox:

obj = {};
obj[{ toString() {throw new Error('nooo')} }] = 10;

throws while console.time({toString() {throw new Error('noooooooo')}}) does not. A closer look shows that the toString() function we made never actually gets called in Chrome. Consider:

...which shows that label.toString() is not accessed via [[GET]](?), yet since the timer was still started and conversion took place, something to the effect of label.__proto__.toString.call(label) must be happening under the hood.

In Firefox this is different, as label.toString() seems to get called and its return value is honored:

If label.toString() throws however, the timer is aborted and the user is never notified:

Sorry if some of those examples are a little beyond the scope of this issue. Are the implementations expected to throw if conversion throws, as opposed to do some sneaky catching under the hood?

I'm learning more about specs & IDL so I checked out the DOMString section of the IDL spec and found that null is not a valid DOMString value. In at least Firefox and Chrome both time(null) and timeEnd(null) work fine; should this ideally throw an error since we are deciding to pass in an "invalid" DOMString? I guess how do we know which is right, the spec or the implementations? Should the spec support something less restrictive than DOMString or do the implementations need to tighten up? I could be misunderstanding some of the specifics here, but it seems like a classic chicken/egg problem in spec land.

One more thing (sorry), in Firefox console.time() never starts a timer whose label = default. Is this likely a Gecko bug that one could file since the spec defines default values for the optional DOMString?

count

In Chrome, count behaves like time in that our custom toString is ignored.

In Firefox, count also behaves similarly to time in that our toString is respected however if it throws the user is never alerted and the label value defaults to "". The result of an erroneous conversion for count in Firefox is the same as a call to count() with no params, and this time the default label value is actually respected.

---

With all this being said, I'm guessing the spec is reasonable and the implementations need to be consistent and possible perform null checking if I understand the IDL spec well enough. Thoughts?

[domenic]

@domfarolino oh, thanks for the great investigation!

The rules for DOMString that are most relevant are the conversion from a JavaScript value. That mostly delegates to the ECMAScript ToString algorithm, which will e.g. convert undefined to "undefined", null to "null", etc. (You can test it in your browser by doing String(x), except that ToString(x) will throw if x is a Symbol, whereas String(x) will not.)

The current IDL, of optional DOMString label = "default", means:

• If given undefined, the label should be "default"
• Any other value should go through the above, and get converted using ToString.

In particular, any exceptions should be rethrown, and any values returned from a toString() method should be used.

It sounds like this is not the case for Chrome, and Firefox does some kind of error-swallowing (although maybe try adding a try/catch block and seeing if it catches anything). The best thing we could do is get more data, e.g. what does Safari tech preview or Edge do. If you write a small test case, e.g. using jsbin, I'm happy to help test in those browsers. If nobody conforms to the current spec, then we should consider loosening it. Otherwise we should work on filing some bugs on browsers.

[domfarolino]

Thanks so much for the information. Quick question about some of the abstract operations. You indicated that the String constructor function closely simulates the ToString abstract operation defined in the ES spec with the exception that it actually converts Symbol => String where the algorithm says to throw an TypeError. With this being said, are there pieces of the language that actually do implement these abstract operations to a T with no discrepancy, or are the abstract operations just there for guidelines, never to be fully implemented under a single function? A lot of them seem to be closely represented by actual language constructs but not sure what the convention/usage is here.

---

time/timeEnd

Default Parameter

This bin can be used to test the correctness of the default parameter currently defined in the IDL. The actual console/devtools will need to be opened in order to see the printing that console.time/timeEnd() does. Note in Chrome when all tests are run in the same window the results MAY be a little deceiving since timers are not removed on calls to timeEnd (see #84). To get around this, each test can be run sequentially in between page refreshes if necessary. Here is what I've found:

• Firefox: timer is never started on calls to console.time() or console.time(undefined), therefore default param is not respected
• Chrome: calling time/timeEnd with no parameters utilizes the correct default label value as the IDL prescribes. Calling console.time/timeEnd(undefined) however, behaves the same as console.time/timeEnd("undefined"). In other words, the default param is NOT triggered and ToString(undefined) is run. I kind of feel this contradicts the purpose of a default parameter at least in JS but could be wrong.
• Chrome Canary: same as above
• Safari: Default parameters are respected, even when undefined is passed in, and all timers work.
• Safari Tech Preview (release 24): same as above ^

DOMString conversion

Again, it might help having DevTools open while running this to see the actual console output. Basic test case in jsbin designed to run console.time with an object whose toString throws an error. A valid implementation will invoke our toString and re-throw the original error it throws. A timer will not be started and a potentially interactive JavaScript representation of the thrown error (result of %o format specifier) should be printed. Results:

• Firefox: calls our toString but swallows errors and timer is not started
• Chrome: doesn't call our toString, does a valid internal conversion and starts a timer
• Chrome Canary: same as ^
• Safari: calls our toString and re-throws error and does not start a timer
• Safari Tech Preview (release 24): same as ^

It seems clear that Safari's implementation most closely follows the spec, if you want to test Edge and whatever else I may be missing (opera?) that would be great! Also, should we standardize at all one what happens when we call console.timeEnd('neverBeforeSeenLabel')? I kind of like how Safari does a Logger('warn', ...'). Chrome seems to print the time in ms, and Firefox does nothing so it is all over the board. Let me know what your thoughts are on proceeding.

[domenic]

Thanks so much for the information. Quick question about some of the abstract operations. You indicated that the String constructor function closely simulates the ToString abstract operation defined in the ES spec with the exception that it actually converts Symbol => String where the algorithm says to throw an TypeError. With this being said, are there pieces of the language that actually do implement these abstract operations to a T with no discrepancy, or are the abstract operations just there for guidelines, never to be fully implemented under a single function? A lot of them seem to be closely represented by actual language constructs but not sure what the convention/usage is here.

The abstract operations are factored-out algorithms that are called from many different parts of the spec. In fact if you go to https://tc39.github.io/ecma262/#sec-tostring, hover your mouse over the heading, and click "References", you can find all the places in the spec that use it. I don't believe there is anything that directly exposes it to a JavaScript-callable function, but many JavaScript-callable functions use it as part of a larger algorithm (e.g. it is the first step in parseFloat/parseInt/decodeURI/... many others).

time/timeEnd Default Parameter

Great test! I ran it in Edge and it matches Safari. So that's great news. We have 2/4 browsers completely spec-complaint, with Chrome almost there. So in this respect I think we should work on filing a couple browser bugs.

time/timeEnd DOMString conversion

Edge gives:

Init
toString being called
Timer "[object Object]" does not exist

This seems kind of similar to Firefox except that it's warning about timeEnd on a non-existant timer, which is I guess a separate issue. I can't tell whether it started a timer, or what name it would use if it did.

So this one's trickier. We have:

1. 2/4 call toString, swallow exceptions
2. 1/4 call toString, rethrow exceptions
3. 1/4 don't call toString, and convert an object to a string in some other way

Of these, (3) seems very strange; let's not do that. Between (1) and (2) it's trickier. I tend to think that (2) is much nicer; it can be specified with Web IDL, and matches the rest of the platform better. I'm not sure if that outweighs the fact that it's outnumbered, but I think since it's only outnumbered 2-to-1 instead of 3-to-1, it probably does. So again I'd tend toward filing bugs on different implementations...

In this particular case, we can also write web platform tests demanding that the exception be rethrown, which is great.

[domfarolino]

Awesome, it's nice this stuff is getting cleaned up a bit. I agree (2) is much better. So we'll need to file the following bugs:

• Chrome time/timeEnd() does not behave like time/timeEnd(undefined): https://bugs.chromium.org/p/chromium/issues/detail?id=696798
• Firefox time/timeEnd() and time/timeEnd(undefined) do not respect the spec's default parameter and no timer is started. This has already been filed (https://bugzilla.mozilla.org/show_bug.cgi?id=1270636)
• Firefox time/timeEnd(obj) do not re-throw exceptions when label.toString() throws: https://bugzilla.mozilla.org/show_bug.cgi?id=1343013
• Edge time/timeEnd(obj) do not re-throw exceptions when label.toString() throws: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11201116/
• Chrome time/timeEnd(obj) never calls label.toString() even though it should. Furthermore it should re-throw in the case where label.toString() throws: https://bugs.chromium.org/p/chromium/issues/detail?id=696805

I can file these this week, mentioning you/this issue for clarification.

You indicated that (2)'s behavior can be specified with Web IDL...is there something we can do in IDL (besides comments) that indicates this function may (re-)throw an exception, or is it all good how it is and the implementations just need to catch up?

---

I'm definitely interested in the web platform tests. I've spent some time reading up on the web-platform-tests repo and testharness.js and have been toying with it a bit this weekend. The console directory seems pretty fresh (only one test file). Are we sticking with a flat directory structure since the spec is relatively small, or would you rather use subdirectories for specific console components? I'll get a PR up soon with some tests elaborating on my previous bins if that sounds ~in the right direction.

[domenic]

You indicated that (2)'s behavior can be specified with Web IDL...is there something we can do in IDL (besides comments) that indicates this function may (re-)throw an exception, or is it all good how it is and the implementations just need to catch up?

This is a pretty standard consequence. Part of the issue is that not all implementations use Web IDL to implement their console, so this needs more careful testing than usual, as you've noticed.

Are we sticking with a flat directory structure since the spec is relatively small, or would you rather use subdirectories for specific console components? I'll get a PR up soon with some tests elaborating on my previous bins if that sounds ~in the right direction.

Flat makes sense to me as there aren't many components, but I'll review over in that PR that I see you opened :D.

---

In general, I wonder if we should split this issue into separate issues for each API? We've spent some good time on this thread working through time/timeEnd, and it might be nice to have a separate thread for finishing count, or for table, or similar.

[domfarolino]

This is a pretty standard consequence. Part....
Makes sense, I figured the throwing would be expected.

I agree with splitting this up. I'll make an issue for count and we can handle table after or something. BTW all bugs have been filed except for Edge, which I can get to this week. Thanks for the help, these are probably small fish for you to fry, so much appreciated. Also, when testing (console) APIs that don't return/throw anything but log something, are we bound to visual tests since we can't programmatically get the last logged console message (I don't think)?

[domenic]

Oh, it's also worth noting that console.assert() has non-any... IDL, so it needs some validation too. E.g. what does { [Symbol.toPrimitive]() { throw new Error("foo"); } } do as a first argument.

[domfarolino]

@domenic Before possibly breaking assert validation into its own thread I want to verify that it might not be needed? Checking out 3.2.3 boolean in Web IDL shows that it largely delegates to ToBoolean() which is actually the end of the chain as far as type coercion goes. This is nice because the table in ES states that just about every possible input has a corresponding primitive output (even objects!). This means console.assert({}) behaves as console.assert(true) which of course is not a failed assertion. In short, @@toPrimitive is never called.

This behavior is consistent on:

• Chrome + Canary
• Safari + Tech Preview
• Firefox
• (I'm guessing edge?)

---

I've filed the last bug (edge) for the time/timeEnd() label conversion, so I assume we close this issue when all implementations catch up to the spec, namely when the filed issues are "closed" right?

[domenic]

Oh, you're totally right; I forgot ToBoolean semantics! Great catch.

I assume we close this issue when all implementations catch up to the spec, namely when the filed issues are "closed" right?

In general it's not the spec's job to track implementation compliance, as long as we've done our homework on filing bugs. However since this is a small spec it might be feasible to keep a tracking document of all bugs we file (similar to, or maybe inside of, NOTES.md) so that people can watch the progress on getting them fixed.

So I think it's OK to close this (and split out table into a separate issue, or maybe use #21).