I took the modifications in NativeJSON.java from this PR, and although I don't fully understand them, they are working fine in some of my tests (JS from reddit.com in gngr).

Without these changes, the JS on reddit fails because it tries to JSONify an array of ScriptStackElement and that fails because ScriptStackElement doesn't have toJSON method. I have yet to find out why the JS needs to JSONify the stack trace but that's a separate concern.

The conflicts in this PR are trivial. They are modifying build.xml and .gitignore. And those modifications don't seem to be essential for this PR. Would be great if @pfn can chip in. I can make a fresh PR without the conflicts, if required.

Just a followup: I analysed the minified JS from reddit. It is calling JSON.stringify on the stack trace because it captures exceptions and sends them to the server. All that is working fine with the changes in this PR (modulo the changes in build.xml).

Thanks for revisiting this -- I'm honestly not sure about this whole PR (the original one) because I'm not sure what exact problem it's trying to solve and as you pointed out, it changes a bit of code.

I am concerned if Reddit has some JS that fails when trying to print the stack trace, especially since we just changed that code in 1.7.6. Can you point to the code? Perhaps we need to tweak the stack trace changes a bit.

The Reddit code is humongous. But here is a one liner:

try { someUndefined() } catch (e) { JSON.stringify(e)}

Sample output:

$ java -jar js.jar
Rhino 1.7.6 2015 04 15
js> try { someUndefined() } catch (e) { JSON.stringify(e)}
js: Java class "[Lorg.mozilla.javascript.ScriptStackElement;" has no public instance field or method named "toJSON".

I've checked 1_7R5 release and it seems that there's the same issue: NativeError cannot be JSON.stringify()-ed because of either its stacktrace (being [Lorg.mozilla.javascript.ScriptStackElement, without toJSON property) or message (it's a String wrapped into NativeJavaObject, so NativeJSON is trying to marshall its fields and getters and fails on getBytes(), as again it returns a [B without toJSON property). So probably it's not related to recent changes.

I'll look more closely on this tomorrow.

Yep, I see that... I think it's something that we have to fix. I think we should move this out of this particular PR, however, and make it a separate issue.

This PR generically allows all java objects to be serializable to JSON; the side-effect is that what @hrj wants now works.

And yeah, I'm not interested in updating it since I'm not using rhino anymore.

@gbrail Thanks for creating the issue #182 for this.

@sainaen et al, I am trying to summarize this PR and the issue, based on my understanding, which may be way off. Please correct me if I have it wrong.

During a call to JSON.stringify when toJSON is not defined for a JS object, the object's properties should be serialized to JSON by default. For a Java object, I guess the properties defined by its Wrapper should be serialized to JSON.

However, this PR gets the properties of a Java object by looking at its bean properties. However, this is very likely not desired. It is the wrapper object that should decide what properties to expose to JS land.

So, the JSON.stringifymethod for an object can be simply defined as:

• If the object being stringified has toJSON, then call that. Else,
• Recursively Stringify every property declared by the object.

If I am atleast somewhere close to understanding this, I can try to create a new PR.

I believe this patch (or something similar to it) is being used in production by the folks at NetSuite, as their Rhino environment happily stringifies Java objects and arrays. It would be great to see something like that land in upstream Rhino.

?is this fixed in the latest release in 2018?

Am closing this PR because:

• support to stringify instances of NativeJavaObject that are instances of Collection or Map has been implemented in Allow JSON.stringify to operate on Java objects #860
• the code that was mentioned above that errored out on trying to stringify the .rhinoException property of a exception no longer fails: try { someUndefined() } catch (e) { JSON.stringify(e.rhinoException)}
• if the approach taken in this PR offers anything more that what got implemented in Allow JSON.stringify to operate on Java objects #860, this PR would have to be completely redone anyway
• the original author is no longer persuiing this PR
• the PR is more then 10 years old