Improvements to Web IDL exception message strings

[Boris Zbarsky]

Hello all,

We just made some changes to how exception strings from ErrorResult get
formatted [1]. The changes apply to both the Throw* DOMException
methods and ThrowTypeError/ThrowRangeError.

In the new setup, exception messages have "%s: " prepended to them,
where %s is replaced with one of:

* "InterfaceName.methodName", when the exception is from a method cal.
* "InterfaceName.attrName getter", when the exception is from a getter
* "InterfaceName.attrName setter", when the exception is from a setter
* "InterfaceName constructor", when the exception is from a constructor.

The idea is to give web developers a bit more context about what callee
failed.

I have tried to go through existing message strings and remove
duplication of that information where it existed.

The changes only affect exceptions thrown via the ErrorResult that
bindings pass to implementation methods. In particular, Promise
rejections via Promise methods (as opposed to throwing on the passed-in
ErrorResult) do not benefit from these changes; more work will need to
happen there.

The changes also do not affect exceptions thrown via
ErrorResult::Throw(nsresult). As a reminder, this is deprecated and
people should switch to the methods that provide useful error messages.

Please let me know if there are concerns around this change, suggestions
for further improvements, etc!

-Boris

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1613013

[Domenic Denicola]

On Friday, February 7, 2020 at 8:55:45 AM UTC-5, Boris Zbarsky wrote:
> Hello all,
>
> We just made some changes to how exception strings from ErrorResult get
> formatted [1]. The changes apply to both the Throw* DOMException
> methods and ThrowTypeError/ThrowRangeError.
>
> In the new setup, exception messages have "%s: " prepended to them,
> where %s is replaced with one of:
>
> * "InterfaceName.methodName", when the exception is from a method cal.
> * "InterfaceName.attrName getter", when the exception is from a getter
> * "InterfaceName.attrName setter", when the exception is from a setter
> * "InterfaceName constructor", when the exception is from a constructor.

Would you consider using something that avoids the confusion with static methods/properties? E.g.

- InterfaceName's methodName()
- InterfaceName's attrName getter
- InterfaceName's attrName setter

As-is it seems like it would be confusing to talk about, for example, Node.appendChild, which is === undefined.

I'm also curious what the output would be for actually-static methods, e.g. CSS.escape(), DOMPointReadOnly.fromPoint(), etc.

[Boris Zbarsky]

On 2/8/20 2:31 PM, Domenic Denicola wrote:
> Would you consider using something that avoids the confusion with static methods/properties? E.g.
>
> - InterfaceName's methodName()
> - InterfaceName's attrName getter
> - InterfaceName's attrName setter

We could think about doing that, yes. If we do, we should change all
the existing messages that talk about InterfaceName.methodName (e.g. the
ones that get thrown for invalid arguments, etc).

The main goal here is to give web developers an idea of which method
they were calling quickly, without taking up space from the actual error
message which tells them what it was they did wrong in calling it. So
there's a tension between brevity and getting the details right... Your
suggestions are pretty decent on the brevity front, though, so that
seems workable.

I wonder what web developers think of the options here...

> I'm also curious what the output would be for actually-static methods, e.g. CSS.escape(), DOMPointReadOnly.fromPoint(), etc.

It'd be "CSS.escape: ", etc.

You can test this now in the console using:

CSS.escape() /* no args */
document.getElementById() /* no args */

My changes did not affect the error messages from those, note; just used
that same string in more places.

-Boris

[Boris Zbarsky]

On 2/10/20 9:57 AM, Anne van Kesteren wrote:
> Would using Document.prototype.getElementById be too long?

That is a good question.

I'm going to do a bit of refactoring so we only need to change one place
to try these ideas out and would love some feedback!

In the meantime, someone who wants to try this locally should apply the
patches in https://bugzilla.mozilla.org/show_bug.cgi?id=1614471 and
experiment a bit.

-Boris

[Simon Giesecke]

Hi,

helpful exception messages are very important, and ensuring that
sufficient context is present is a good contribution to that. Thanks
for pushing this forward!

When starting work on replacing uses of the deprecated
ErrorResult::Throw(nsresult), I noticed that the existing error
messages show a syntactical inconsistency. Some end with a period (.),
some do not. To ensure a consistent presentation, we should define the
preferred style for this, and apply it consistently.

I think the options with their rationale are:

1. individual messages passed to Throw* should not end in a period,
but should form a complete sentence otherwise and a period suffix
might be added depending on how/where it is exposed

2. all individual messages passed to Throw* should end in a period,
and they form a complete sentence; no additional period is ever added
regardless of how they are exposed

3. individual messages passed to Throw* may or may not end in a
period, depending on whether they form a complete sentence; no
additional period is ever added regardless of how they are exposed

I think either of the first two options should be chosen, with a
slight preference for the first option.

Are there other opinions on this? Maybe there are more options I
didn't consider.

Simon

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[Anne van Kesteren]

On Mon, Feb 10, 2020 at 3:23 PM Boris Zbarsky <bzba...@mit.edu> wrote:

> You can test this now in the console using:
>
> CSS.escape() /* no args */
> document.getElementById() /* no args */
>
> My changes did not affect the error messages from those, note; just used
> that same string in more places.

Would using Document.prototype.getElementById be too long? I've
sometimes seen developers advocate using # to mean .prototype (e.g.,
Document#getElementById via
https://mathiasbynens.be/notes/javascript-prototype-notation), but I
don't know how widespread that is. It does seem a little funky to use
Document.getElementById.