378281 - JS_ReportErrorNumber doesn't use exception type from supplied JSErrorCallback

Status: RESOLVED DUPLICATE of bug 215173

(Core :: JavaScript Engine, defect)

Description

[Alexey Naidyonov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8.1.3) Gecko/20070310 Iceweasel/2.0.0.3 (Debian-2.0.0.3-1)
Build Identifier: 

JS_ReportErrorNumber(UC) takes JSErrorCallback and userRef as parameters, but doesn't pass these to js_ErrorToException. In turn, js_ErrorToException calls default js_GetLocalizedMessage using errorNumber from supplied JSErrorReport, so the resulting exception type is taken from system JSErrorFormatString table, from the record of error with the same number as the user supplied one. This is obviously wrong, and could even lead to smth bad happened if the user's JSErrorFormatTable size will exceed system's one.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Brendan Eich [:brendan]]

This is fixed on the CVS trunk already. Please search all bugs (including closed state) for key function names and keywords before filing. Thanks,

/be

Comment 2

[Alexey Naidyonov]

Do I get it correctly that http://lxr.mozilla.org/mozilla/source/js/src/jsexn.c points to a trunk version? Than I still think there is an inconsitency in the API. JS_ReportErrorNumber takes callback and pointer to user supplied data, and use these two to figure out format:

if (!callback || callback == js_GetErrorMessage)
    efs = js_GetLocalizedErrorMessage(cx, userRef, NULL, errorNumber);
else
    efs = callback(userRef, NULL, errorNumber);

Than it calls js_ErrorToException, which does exactly this:

errorString = js_GetLocalizedErrorMessage(cx, NULL, NULL, errorNumber);

js_GetLocalizedErorMessage uses JSErrorCallback supplied as JSLocaleCallbacks, but:
1. It doesn't pass userRef (if it's used)
2. Locale callbacks are context-wide, while there are might be few modules using JS_ReportErrorNumber facility (e.g., my module and jsfile.c module)

Please get me right, I trying to fugure out the correct pattern to use exceptions in SpiderMonkey. I'd be very grateful for any hints (e.g., how can I checkout trunk if it's really fixed there)

Comment 3

[Brendan Eich [:brendan]]

userRef is passed through:

    if (cx->localeCallbacks && cx->localeCallbacks->localeGetErrorMessage) {
        errorString = cx->localeCallbacks
                        ->localeGetErrorMessage(userRef, locale, errorNumber);
    }

(from js_GetLocalizedErrorMessage).

This is all because of API compatibility. The original design flaw is quite old and hard to dig out, so the solution Michael Daumling and I agreed on adds a new localeCallback. Add another layer, keep backward compatibility while allowing for l10n of all messages.

If I'm missing something here, and userRef is not propagated, please file a new and specific bug. Thanks,

/be

Comment 4

[Alexey Naidyonov]

That's correct, userRef is not passed to second call of js_GetLocalizedErrorMessage in js_ErrorToException.

> please file a new and specific bug

I'll do so.