1408013 - Uncatchable exception setup is a footgun

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

SpiderMonkey has a concept of uncatchable exceptions (e.g. thrown by interrupt callbacks), which is implemented by returning null or false from a JSAPI call but not storing an exception on the JSContext.

This is a serious footgun in practice, because it makes it very easy to "lose" the uncatchability of the exception unless one is extremely familiar with how JSAPI works.  See discussion in bug 1407669 for example, and this is a pretty common occurence.

It might be nice if uncatchable exceptions that are meant to be uncatchable explicitly set a flag on the JSContext.  That is, if "returned false/null but there is no indication of failure on the JSContext" were considered a bug in SpiderMonkey APIs.

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Boris Zbarsky [:bzbarsky] from comment #0)

It might be nice if uncatchable exceptions that are meant to be uncatchable
explicitly set a flag on the JSContext. That is, if "returned false/null
but there is no indication of failure on the JSContext" were considered a
bug in SpiderMonkey APIs.

After bug 1921215, bug 1921780, and bug 1921963, uncatchable exceptions now require a JS::ReportUncatchableException call that sets a flag on the context. A plain return false; without reporting anything will result in an assertion failure in debug builds, if we haven't reported any uncatchable exception before.

The main thing missing still is that the flag is "context has at some point reported an uncatchable exception" and not "context is currently throwing an uncatchable exception", so the tracking is not as precise as it could be. This will still let us catch most bugs though.

For precise tracking we'd have to integrate with the ExceptionStatus enum. Maybe we can keep this bug open for that part.