Add ScriptRuntime:throwCustomError to make it easier to re-throw Java exceptions

[raimi]

I maintain a largish application with lots of java/rhino-interwoven code. Luckily we have a test suite wich showed us that something broke with 1.7.6 wich still worked with 1.7R5.
I can reproduce serveral occasions of this trace:

java.lang.IllegalArgumentException: No enum constant org.mozilla.javascript.TopLevel.NativeErrors.XXX
    at java.lang.Enum.valueOf(Enum.java:236)
    at org.mozilla.javascript.TopLevel$NativeErrors.valueOf(TopLevel.java:67)
    at org.mozilla.javascript.ScriptRuntime.newCatchScope(ScriptRuntime.java:3439)
    at org.mozilla.javascript.gen...

where XXX is a custom js-class that I use like "throw new XXX()". I suspect these commits:
9320d8c
15be937
ff0ef45

Unfortunately, I cannot reproduce the problem in a simple Rhino shell. This all may be my fault, as I use dynamic scopes and re-loading of js source files. I just post here to warn others for a possible change of semantics.

The above Exception occurs with this code:

try {
 foo();
} catch (e) { }

...when foo() throws one of my XXX. Somehow it is an EcmaError here: https://github.com/mozilla/rhino/blob/master/src/org/mozilla/javascript/ScriptRuntime.java#L3436 ...but that does not work out.

Does this ring a bell for somebody?

[raimi]

PS: I dremel'd something by reverting the suspicous commits: https://github.com/raimi/rhino/commit/219c06a6f30ea2afbdc7173cf90e945164ceca3c

This fixes my problem but is wrong with regard to that one exception being thrown. https://github.com/raimi/rhino/commit/219c06a6f30ea2afbdc7173cf90e945164ceca3c#diff-5461f4051c38ef65beb8b329a22ad442R536

Heeeelp :)

[sainaen]

@raimi
Could please describe in more details where are your exceptions coming from?
For me the main question is: how are your XXX exception is an instance of EcmaError.

To check that you don't create EcmaError in your java code, can you please apply this commit and run your tests again? There I replaced the names of EcmaError in strings with values from NativeErrors enum.

[raimi]

a-ha, that's it. With your commit applied my java code doesn't compile anymore :)

I have an instance of "throw ScriptRuntime.constructError(className, msg);" in my code. That's the culprit.

I wasn't aware of that situation: The Java code tries to throw an Exception object that is defined in the JavaScript world - and all of that has to be mangled through the Java API...

Bottom line:

0. It's my fault as I'm throwing wrongly.

1. The API change itself was neccessary as it fixes a Spec-compatibility issue. I contaced André Bargull (the original author of the commit) off-list and he pointed that out to me.

2. Your patch should be applied as it reveals this kind of problem at compile time. There is now an implicit contraint on some String arguments which should be explicit.

Thanks, Internet!

[raimi]

Dear Internet, @sainaen , @gbrail !

I think that I figured out what the problem is: For ages I've been throwing custom-defined JS-Objects (called "Exception" and some sub-types) from the Java side. Those objects behaved like native "Error" objects (duck typing), at least with regards to what I wanted from them.

@anba's commit disallowed the throwing of arbitrary objects for good reason, @sainaen's commit makes sure this is enforced in the java-world.

I hereby try to contribute a little patch: https://github.com/raimi/rhino/commit/b3234c4fe3280a204da031a0b7c70a5b11dcb600 - in this branch together with the other stuff: https://github.com/raimi/rhino/commits/issue_176

This introduces a java-way of throwing js-objects, given their constructor name (same as the very old implementation used to allow). It uses the same arguments, so that an implementation has access to the source position etc.

My test suite is still running, i'll see if this breaks anything else in existing code :)

Am I missing something or may this be the right thing to do?

[gbrail]

Yes, I think that opening up a pull request is a good way to handle this. The change looks reasonable to me.

[sainaen]

@raimi
I'm glad I could help. But, please, don't merge my commit. What I've done there is a bit hacky in some places and should be tied up before merging.
As for your change, I'm fine with adding new method, but I think JavaDoc should be updated, as it is not an equivalent of new Error(message), because we expect a custom constructor name here.

[raimi]

Ok, thanks for your comment, @sainaen ! I created a pull request for @gbrail with only my small method and corrected javadoc: #181
Can this be Rhino 1.7.7?

[gbrail]

Thanks for tracking this down and sticking with it!