306634 - Request: use JSREPORT_WARNING flag to talk back to engine

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, enhancement)

Description

[Michael Daumling]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6

The ErrorReporter callback does not have a value to indicate whether it would
like the script to be terminated because of a warning. If the ErrorReporter
could clear the JSREPORT_WARNING flag, the caling function (2 of them) could
treat the warning as error and terminate the script. This feature would allow
for the treatment of individual warnings as errors.

Reproducible: Always




The change won't break backwards compatibility, but until now, the JSErrorReport
supplied to the error reporter has been considered as read only.

js_ReportErrorNumberVA() and js_ReportErrorVA() would have to be modified - the
code to be instered after the call to ReportError() is:

if (!(report.flags & JSREPORT_WARNING))
    warning = JS_FALSE;

Comment 1

[Brendan Eich [:brendan]]

Michael, feel free to take this bug and patch it.

Readonly-ness of JSErrorReport has not been enforced by a type system or any such thing, so it's possible that fixing this bug could break backward compatibility for some users who mutate members (for whatever reason) including flags.  But it's probably safe to do this and argue that such users were not forward compatible in their mutations of JSErrorReport.  Anyway, I don't expect much breakage, if any.

/be

Comment 2

[Michael Daumling]

Hmm...

Unless someone else asks for it, I'd like to withdraw this one. It has been raised as an issue early on in my integration process, and I do not really need this functionality anymore. Also, some errors would probably leave the engine in an instable state if they would be converted to warnings, which could bring down the entire system.

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #2)
> Hmm...
> 
> Unless someone else asks for it, I'd like to withdraw this one. It has been
> raised as an issue early on in my integration process, and I do not really need
> this functionality anymore. Also, some errors would probably leave the engine
> in an instable state if they would be converted to warnings, which could bring
> down the entire system.

Warnings may become errors with the JSOPTION_WERROR option, but not vice versa, you're right.  And error reporter calls generally count on throwing or reporting OOM, and they propagate false or null as an unambiguous failure return value.  So if this bug were fixed naively, it would result in warning reports followed by unavoidable failures.

The deeper fix would have to add more return value sensing to (JS|js)_Report* call sites, which would be a big patch and a further API incompatibility.  So I happily agree: WONTFIX.

/be