244492 - JavaScriptException to extend RuntimeException and common exception base

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, enhancement)

Description

[Igor Bukanov]

Currently Rhino runtime can throw 3 different types of exceptions:

1. JavaScriptException that is thrown by JavaScript throw statement.
2. EcmaError that represents JavaScript runtime exception as specified by ECMA
standard.
3. EvaluatorException that is thrown on compilation errors and as a wrapper for
exceptions in Java functions.

Since JavaScriptException extends Exception, it has to be always declared in the
method signature. In principle this useful to provide a hint that a particular
API can execute some JavaScript code. But in practice it does not work this way
since virtually any Rhino API method can cause indirect execution of JavaScript
code and in most such cases the API does not have "throws JavaScriptException"
declaration.

For example, Context.toString(Object) executes toString function if it is
defined for the object and it can be defined with JavaScript. In addition any
native method including getters and setters can trigger JS execution and "throws
JavaScriptException" is not included in Scriptable.get/Scriptable.put methods.

For this reason I suggest to change JavaScriptException to extend from
RuntimeException so it would not be necessary to wrap it into artificial
wrappers to circumvent lack of the exception declaration.

Such change will cause source compatibility error if Java code contains:

try {
    ....
} catch (RuntimeException ex) {
    ...
} catch (JavaScriptException ex) {
    ...
}

but this is trivially fixable in a forward and backward compatible way by
changing statement's order. 

In addition for compiled classes with such code catch (JavaScriptException ex)
would not be reached so it may change program's semantics but at least the
change would be binary compatible.

With such change in place it would also make change to introduce a common base
class for all exceptions thrown by Rhino so it would be simple to process all
such exceptions.

Comment 1

[Igor Bukanov]

Attachment: JavaScriptException extends RuntimeException

The patch changes JavaScriptException to extend from RuntimeException and
removes various catch blocks for JavaScriptException that rethrow exception or
ignore it since it was not supposed to happen.

Comment 2

[Igor Bukanov]

I committed attachment 149204 [details] [diff] [review]

Comment 3

[Igor Bukanov]

Attachment: RhinoException as common exception root

The patch adds RhinoException as a common root for all exceptions for Rhino.
The class contains method to init/get script source location for the exception
which is used by the rest of Exception classes.

To preserve binary compatibility I preserved the original source information
methods as a deprecated form.

Comment 4

[Igor Bukanov]

I committed the last patch.

Comment 5

[Igor Bukanov]

Proper target milestone

Comment 6

[Igor Bukanov]

The common root exception changes included also deprecation of PropertyException
since in common cases they were caught and rethrown as runtime exceptions by
runtime or just caught and ignored. So I replaced throwing of PropertyException
by  Context,reportRuntimeException and deprecated the class. 

But that causes a compatibility problem during compilation since
PropertyException extends Exception and no longer explicitly declared in Rhino
API. So code containing catch (PropertyException ex) may not compile. To fix
that I commited a patch to extend PropertyException from RuntimeException to
have less suprises during Rhino upgrade.