Closed Bug 518621 Opened 13 years ago Closed 13 years ago

JS_ReportErrorNumber ignores exception type for user-generated messages

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dwitte, Assigned: dwitte)

Details

Attachments

(1 file)

So, you're supposed to be able to construct your own JSErrorFormatString, consisting of { format, count, exception }, which JS_ReportErrorNumber() fetches via your own defined my_GetErrorMessage() function. However, what it really does is compare whether the supplied fnptr is js_GetErrorMessage():

http://mxr.mozilla.org/mozilla-central/source/js/src/jscntxt.cpp#1421

and does localization stuff etc. It seems to get this part wrong. It uses the format and count just fine, but uses the exception type from http://mxr.mozilla.org/mozilla-central/source/js/src/js.msg. So, if I want a TypeError, I have to make my error number 3.

This, while being somewhat amusing, is also pretty bizarre.
Attached patch patch v1Splinter Review
This is the minimal fix; it propagates the callback fnptr into js_ErrorToException so it can do a proper lookup of the exception type, instead of assuming js_GetLocalizedErrorMessage.

I thought about doing a more extensive fix. The problem is really that JSErrorReport (https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JSErrorReport) has an errorNumber field, but that means nothing without a callback to go with it. But this is part of JSAPI and is used by user error reporters and debug hooks, so they're probably already assuming that errorNumber corresponds to a jsengine error in js.msg. We could perhaps add an exnType field, which either gets hardcoded to the right value (for jsengine errors) or filled in by js_ExpandErrorArguments when it calls the callback.
Assignee: general → dwitte
Attachment #415766 - Flags: review?(mrbkap)
Attachment #415766 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/mozilla-central/rev/760590721612
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.