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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla45
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.
![]() |
Assignee | |
Comment 1•10 years ago
|
||
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...
![]() |
Assignee | |
Comment 2•10 years ago
|
||
bkelly had the excellent idea of having the API be rv.NoteJSContextException().
![]() |
Assignee | |
Comment 3•10 years ago
|
||
Attachment #8680643 -
Flags: review?(peterv)
![]() |
Assignee | |
Updated•10 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 4•10 years ago
|
||
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+
Comment 6•10 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•