Some changes to how errors are thrown from Web IDL methods

Hello all,

I just checked in some improvements to how we report exceptions on 
ErrorResult and in promise rejections [1].  Summary:

1) The public ThrowDOMException method that takes an nsresult and a 
string is gone.  Instead, there are methods like ThrowIndexSizeError, 
ThrowHierarchyRequestError, etc, matching the error names defined at 
<https://heycam.github.io/webidl/#idl-DOMException-error-names>.  So 
whenever the spec says to "throw an SecurityError exception" or whatnot, 
you call ThrowSecurityError.  These methods can take either a string 
literal ("foo") or any nsACString.  nsPrintfCString can be used to 
produce error messages that depend on the input to the DOM method.

2) The public Promise::MaybeRejectWithDOMException method is gone. 
Instead there are MaybeRejectWithIndexSizeError, etc, methods.  Again, 
these just take the string to use for the message as input.

3) While ErrorResult::Throw taking just an nsresult still exists, it is 
deprecated and new code should not be adding new calls to it if that can 
be avoided.  Callsites that pass a constant nsresult value can be 
changed to use the above methods with meaningful error messages; I will 
be doing some of that, but would appreciate help from subject matter 
experts who can probably write better error messages than I can.  Same 
thing for ErrorResult::operator=(nsresult).

4) Promise::MaybeReject(nsresult) still exists, but is deprecated. 
Please try to convert to one of the above methods or taking an 
ErrorResult that had a meaningful exception thrown on it.

I would really like to get to the point where when web developers see 
errors in their console they don't have to guess what caused those 
errors, and having meaningful messages is the simplest way to get there.

-Boris

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1612213
0
Boris
2/3/2020 9:25:09 PM
mozilla.dev.platform 6600 articles. 0 followers. Post Follow

4 Replies
25 Views

Similar Articles

[PageSpeed] 17

On Thu, Feb 6, 2020 at 9:12 AM Boris Zbarsky <bzbarsky@mit.edu> wrote:

> 3) While ErrorResult::Throw taking just an nsresult still exists, it is
> deprecated and new code should not be adding new calls to it if that can
> be avoided.
>

We are attempting to add a static analysis that blocks new uses of
`NS_NewNamedThread` in bug 1613440 [0].  We'd definitely like to consider
making it general enough that it can serve as a sort of
quasi-[[deprecated]] [1] for other uses in the code base (e.g. the
sandboxing folks would like to incrementally disallow functions from being
called from certain locations as they work to lock down the sandbox), and
this sort of replacement seems like another good application.

If you have other things you'd like this static analysis to be used for,
please file dependencies on bug 1613440.

Thanks,
-Nathan

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=1613440
[1] We can't actually use [[deprecated]] / __attribute__((deprecated))
because of their use in third-party code; having the compiler error on uses
of such functions would break the build. [2]
[2]
https://searchfox.org/mozilla-central/rev/3811b11b5773c1dccfe8228bfc7143b10a9a2a99/build/moz.configure/warnings.configure#140-142
0
Nathan
2/7/2020 3:13:58 PM
On 2/7/20 10:13 AM, Nathan Froyd wrote:
> If you have other things you'd like this static analysis to be used for,
> please file dependencies on bug 1613440.

Nice!  I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1614548

-Boris
0
Boris
2/11/2020 4:29:33 AM
On Thu, Feb 6, 2020 at 3:12 PM Boris Zbarsky <bzbarsky@mit.edu> wrote:
> I would really like to get to the point where when web developers see
> errors in their console they don't have to guess what caused those
> errors, and having meaningful messages is the simplest way to get there.

This is a great goal and we should definitely improve our error
messages, but I continue to be worried about exposing more data there
than is advisable from a security/privacy standpoint. In particular as
from a developer ergonomics standpoint it can be hugely valuable to
include such data. (Since I raised this last time we actually had a
security bug related to this.)

I don't know how much work this is, but ideally the signature is
something like throwType(safeMessage, consoleMessage), whereby
consoleMessage defaults to safeMessage or some such. This would allow
for exposing confidential data to developers when debugging locally
and keeps user data secure/private (assuming all callers are holding
it correctly and get reviewed as such).
0
Anne
2/11/2020 1:57:23 PM
On 2/11/20 8:57 AM, Anne van Kesteren wrote:
> I don't know how much work this is, but ideally the signature is
> something like throwType(safeMessage, consoleMessage), whereby
> consoleMessage defaults to safeMessage or some such.

We can certainly do that.  It's "some" work for DOMException: adjusting 
the API callsites to accept the extra arg, adjusting the internal 
ErrorResult storage to store it, adjusting DOMException to store it, 
exposing a [ChromeOnly] accessor on DOMException.  I can throw this 
together pretty quickly.  Then devtools would needto use the new accessor.

For JS exceptions it would require SpiderMonkey-side changes which would 
be a lot more involved, as far as I can tell, including changes to the 
generally less-flexible "throw a TypeError" API we have to work with. 
I'd want someone on the SpiderMonkey team to do this, ideally, because 
I'm not sure which parts of this setup we really want and which we can 
throw overboard..

The hard part, of course, is knowing when to do the separate 
consoleMessage thing...

-Boris
0
Boris
2/11/2020 3:58:55 PM
Reply: