xpconnect should allow nsIException in places where it requires nsIXPCException

VERIFIED FIXED in mozilla1.0

Status

()

Core
XPConnect
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: John Bandhauer, Assigned: John Bandhauer)

Tracking

Trunk
mozilla1.0
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
When nsIXPCException was factored out into nsIException to allow for generic
exceptions, some places in xpconnect were not updated to reference the base
nsIException interface as they should have been. This breaks the case where
non-nsIXPCException wrapped native nsIExceptions are passed through xpconnect.
XPConnect should correctly unwrap them (for error reporting and the like).
Instead, it fails to QI them to nsIXPCException and so it thinks they are just
plain nsISupports objects - even though they are perfectly useful implmenttions
of nsIException.

This problem bit rayw's SOAP code. I've also heard reports of other problems
with unrecognised exceptions that I think resolve to this problem.

I see that another place where we should have converted to nsIException is in
the code that handles JS code like: "if(foo instanceof Components.Exception)..."

I'll attach a patch that should fix both of these problems.

The following is from an email from rayw that illustrates the problem he had
with getting a useless error report where his very helpful error report should
have been emitted....

1.  I have code in SOAP for throwing exception objects with long descriptions on
many SOAP errors to help the JS writer.  This calls SetCurrentException on a
fairly-well-implemented nsIException, which has the appropriate class info and
works fine in most JS code, but when it is called from a callback such as the
SOAP responseHandler, instead of reporting the nice error, it gets strange.  I
am sure I should have reported this as a bug, and I will file a bug soon, but
I'd like to get an idea of what is wrong.

The latest version of the soapisprimenumber.html test relies on
soapisprimeproxy.js in the same directory.  If I go to line 22 and change the
word "boolean" to float", I get the error:

Error: [Exception... "'Javascript component threw a native object that is not an
exception' when calling method: [nsISOAPResponseListener::handleResponse]"
nsresult "0x8057001b (NS_ERROR_XPC_JS_THREW_NATIVE_OBJECT)" location "<unknown>"
data: yes]

If I eliminate the oncompletion argument on line 35 of the same file to make the
call synchronous instead of asynchronous, then I get the proper error message
that the user should get:

Error: uncaught exception: SOAP_TYPE_SUBCLASS: The type of the element or
xsi:type must be a subclass of the required type, called by JS frame ::
file:///home/rayw/mozilla/extensions/xmlextras/tests/soapisprimeproxy.js ::
anonymous :: line 30
(Assignee)

Comment 1

16 years ago
Created attachment 75519 [details] [diff] [review]
proposed fix
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: mozilla1.0, patch
Target Milestone: --- → mozilla1.0
Comment on attachment 75519 [details] [diff] [review]
proposed fix

sr=jst
Attachment #75519 - Flags: superreview+

Comment 3

16 years ago
*** Bug 132753 has been marked as a duplicate of this bug. ***

Comment 4

16 years ago
The patch works as advertised.  It causes SOAP to display the correct messages.
 I assume that nsIXPCEception is still needed for internal use, which is why
there are still a few references to it in this directory (but nowhere else)
after the patch is applied.

Comment 5

16 years ago
Comment on attachment 75519 [details] [diff] [review]
proposed fix

r=dbradley

Looks good to me. From what I saw the remaining instances are correct.
Attachment #75519 - Flags: review+

Comment 6

16 years ago
Comment on attachment 75519 [details] [diff] [review]
proposed fix

a=scc
Attachment #75519 - Flags: approval+
(Assignee)

Comment 7

16 years ago
Fix checked in. Thanks for the quick review approval and testing.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 8

16 years ago
Marking Verified -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.