Closed Bug 1219749 Opened 10 years ago Closed 10 years ago

Add a way to "throw from JSContext" on ErrorResult

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox44 --- affected
firefox45 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

We have a somewhat-common pattern in our tree that looks like this: void SomeIDLMethod(JSContext* cx, ErroResult& rv) { if (!JS::DoSomething(cx)) { // DoSomething threw, exception already on cx rv.Throw(NS_ERROR_FAILURE); return; } } This is all fine and dandy if DoSomething threw an actual exception. But if DoSomething threw an _uncatchable_ exception (e.g. via the slow script dialog) then the above code will cause the bindings to throw an NS_ERROR_FAILURE XPConnect exception, which is catchable. That's busted. I see two ways of fixing this. Either we add a ThrowFromJSContext method on ErrorResult which basically checks whether there is a pending exception and if so throws NS_ERROR_FAILURE, otherwise throws NS_ERROR_UNCATCHABLE_EXCEPTION. Or we pick an nsresult value which explicitly means "don't put a pending exception on the JSContext; if there is one on there now, great, if not, also great". Now that I write it down, I like the second option better.
Note that we have the existing an unused NS_ERROR_XPC_JS_THREW_EXCEPTION, but I'd rather not use it here because of the "XPC" bit...
bkelly had the excellent idea of having the API be rv.NoteJSContextException().
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8680643 [details] [diff] [review] Add a way to faithfully propagate the "exception is already on JSContext" state through an ErrorResult Review of attachment 8680643 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/base/ErrorList.h @@ +548,5 @@ > ERROR(NS_ERROR_DOM_DOMEXCEPTION, FAILURE(1017)), > > + /* An nsresult value to use in ErrorResult to indicate that we > + * should just rethrow whatever is on the JSContext (which might be > + * nothing if an uncatchable exception was thrown. Looks like there's an extra space after the *.
Attachment #8680643 - Flags: review?(peterv) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: