Make stack property available on new Error() too

[mguillem]

The exception thrown by something like:

null.foo()

has the stacktrace available as stack property but the exception thrown by

throw new Error()

doesn't.

This pull request fixes this problem (including unit test)

[pgbakker]

I was looking for a way to get the stack property on custom errors and came up with this, which is a bit more generic, I think, as it exposes the stack property on new Error() but also on custom errors through constructor functions that have Error as their prototype.

Unfortunately I'm not yet fleut enough in git(hub) yet to clone the repo and setup a pullrequest, so below the new code for existing methods/new method for both the JavaScriptException and NativeError class.

====JavaScriptException.java====

    public JavaScriptException(Object value, String sourceName, int lineNumber) {
        recordErrorOrigin(sourceName, lineNumber, null, 0);
        this.value = value;
        // Fill in fileName and lineNumber automatically when not specified
        // explicitly, see Bugzilla issue #342807

        if (value instanceof Scriptable && Context.getContext().hasFeature(Context.FEATURE_LOCATION_INFORMATION_IN_ERROR)) {
            Scriptable obj = (Scriptable) value;
            while(obj != null && !(obj instanceof NativeError)) {
                obj = obj.getPrototype();
            }
            if (obj != null) {

                NativeError error = (NativeError) obj;
                if (!error.has("fileName", error)) {
                    error.put("fileName", error, sourceName);
                }
                if (!error.has("lineNumber", error)) {
                    error.put("lineNumber", error, Integer.valueOf(lineNumber));
                }
                // set stack property, see bug #549604
                error.setStackProvider(this);
            }
        }
    }

=======NativeError.java=======

    public void setStackProvider(RhinoException re) {
        // We go some extra miles to make sure the stack property is only
        // generated on demand, is cached after the first access, and is
        // overwritable like an ordinary property. Hence this setup with
        // the getter and setter below.
        if (stackProvider == null) {
            stackProvider = re;
            try {
                defineProperty("stack", this,
                        NativeError.class.getMethod("getStack", Scriptable.class),
                        NativeError.class.getMethod("setStack", Scriptable.class, Object.class),
                        0);
            } catch (NoSuchMethodException nsm) {
                // should not happen
                throw new RuntimeException(nsm);
            }
        }
    }

    public Object getStack() {
        return getStack(this);
    }

    public Object getStack(Scriptable obj) {
        while(obj != null && !(obj instanceof NativeError)) {
            obj = obj.getPrototype();
        }
        NativeError er = (NativeError) obj;
        Object value = er.stackProvider == null ? NOT_FOUND : er.stackProvider.getScriptStackTrace();
        // We store the stack as local property both to cache it
        // and to make the property writable
        setStack(obj, value);
        return value;
    }

    public void setStack(Object value) {
        setStack(this, value);
    }

    public void setStack(Scriptable obj, Object value) {
        while(obj != null && !(obj instanceof NativeError)) {
            obj = obj.getPrototype();
        }
        NativeError er = (NativeError) obj;
        if (er.stackProvider != null) {
            er.stackProvider = null;
            er.delete("stack");
        }
        er.put("stack", this, value);
    }

Does this make sense?

For some of the discussion that came before this patch, see: https://groups.google.com/forum/#!topic/mozilla-rhino/5p_-9g01eqE

[pgbakker]

Note that this fix puts Rhino inline with browsers that also allow creating custom Errors that then expose the stack property (albeit the stack property not being ECMA spec, most browsers have implemented it to my knowledge)

[katangagonzalez]

Why is this change not included into official rhino version? I'm still not able to get stack of error when throwing and catching the exception

var error = new Error('Error string'); try { throw error; } catch (e) { internal.log(e.stack); }

where internal.log is a function in Java code that logs elements, but I'm still receiving an undefined object.

[gbrail]

Are you running with the debug flag turned on in the Rhino context? Errors only have stack traces when Rhino's debug flag is enabled. For instance:

rhino -e "var error = new Error('Error string'); try { throw error; } catch (e) { print(e.stack); }"
undefined

rhino -debug -e "var error = new Error('Error string'); try { throw error; } catch (e) { print(e.stack); }"
at :1

[p-bakker]

Why would the stack property only be available when the -debug option is set?

[gbrail]

Rhino has historically not put debug information (file names and line
numbers) into its generated class files. I don't know how long that has
been the case but seeing that the first commit to the repo was in 1996 it's
been a while. It is possible that whomever made that decision can tell us.
Given the wide number of places where Rhino is used, it is likely that
someone somewhere is depending on the fact that this was done because it
results in generation of less bytecode.

Without that information in the generated code, there's no way to generate
a JavaScript stack trace.

BTW when I say "turn debug on," I mean to set the "GeneratingDebug" flag on
the Context class. This is what the "-debug" flag on the "rhino"
command-line shell does for instance.

On Fri, Sep 2, 2016 at 7:56 AM, Paul Bakker notifications@github.com
wrote:

Why would the stack property only be available when the -debug option is
set?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#109 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAf0a5_ayAXB5apdyCUwN_SN7dDuOBhbks5qmDkpgaJpZM4Acyut
.

Greg Brail | apigee https://apigee.com/ | twitter @gbrail
http://twitter.com/gbrail @apigee https://twitter.com/apigee

[p-bakker]

ok, that makes sense, didn't think of that.

So, if the stack property is exposed when the GeneratingDebug flag is switched on and its not likely that the stack property will start returning anything while that is not the case, shouldn't this issue be closed then?

[shturec]

I don't think this works as described.

Setting the generatingDebug flag on context to true had no effect for me, and looking (briefly) at the code, I couldn't find relation between the code injecting the stack property and the generatingDebug flag. On the other hand the FEATURE_LOCATION_INFORMATION_IN_ERROR feature seems to be the one to guard the lineNumber, fileName and stack properties injection .

So my goal was to throw a js Error, catch it, and access the call stack property.

The way I managed to achieve that was by enabling the FEATURE_LOCATION_INFORMATION_IN_ERROR (which is off by default) on the global Context factory. There might be more subtle ways of doing that (I'm new to Rhino) but what I did was I one-time preset:

ContextFactory.initGlobal(new ContextFactory() {
	@Override
	protected boolean hasFeature(Context cx, int featureIndex) {
		if (featureIndex == Context.FEATURE_LOCATION_INFORMATION_IN_ERROR) {
			return true;
		}
		return super.hasFeature(cx, featureIndex);
	}
});

Suggestions for better way to enable it are welcome.
In my case I'm using context.evaluateReader and I can do what i intended just fine. Now catching both: throw Error('test') and new Error('test') features a stack property on the error variable defined in the catch block. For example:

try{
	throw Error('test');
} catch(e){
	java.lang.System.out.println(e.stack)  
}

outputs the following to the System.out: at a/b/test.js:1
(assuming script path is a/b/test.js and the code above is its (top) content)
What you can't do is throw a free form object (throw {"message":"test"}) and have the stack property available. I personally, can live with that.

If you are interested in the whole stack, including the java part, there is a 'magic' object injected in the catch scope, so when you are in a catch block you can reference it by the name __exception__. This is a reference to the JavaScriptException wrapper for the thrown Error and you can invoke methods on it as usual.

I hope this helps to those of us in the same boat.

[gbrail]

I honestly don't know why "FEATURE_LOCATION_INFORMATION_IN_ERROR" isn't the default. Maybe it'd have some small performance impact on the code. But if there's support out there for changing the default then we can certainly do that and avoid confusing new users.

…

On Tue, Oct 24, 2017 at 8:28 AM, shturec ***@***.***> wrote: I don't think this works as described. Setting the generatingDebug flag on context to true had no effect for me, and looking (briefly) at the code, I couldn't find relation between the code injecting the stack property and the generatingDebug flag. So my goal was to throw a js Error, catch it, and access the call stack property. The way I managed to achieve that was by enabling the FEATURE_LOCATION_INFORMATION_IN_ERROR <https://github.com/mozilla/rhino/blob/6420e464a890faba5a47ead45e6f1d5050f95260/src/org/mozilla/javascript/JavaScriptException.java#L44> (which is off by default) on the global Context factory <http://rhino/src/org/mozilla/javascript/ContextFactory.java>. There might be more subtle ways of doing that (I'm new to Rhino) but what I did was I one-time preset: ContextFactory.initGlobal(new ContextFactory() { @OverRide protected boolean hasFeature(Context cx, int featureIndex) { if (featureIndex == Context.FEATURE_LOCATION_INFORMATION_IN_ERROR) { return true; } return super.hasFeature(cx, featureIndex); } }); Suggestions for better way to enable it are welcome. In my case I'm using context.evaluateReader and I can do what i intended just fine. Now catching both: throw Error('test') and new Error('test') features a stack property on the error variable defined in the catch block. For example: try{ throw Error('test'); } catch(e){ java.lang.System.out.println(e.stack) } outputs the following to the System.out: at a/b/test.js:1 (assuming script path is a/b/test.js and the code above is its (top) content) What you can't do is throw a free form object (throw {"message":"test"}) and have the stack property available. I personally, can live with that. If you are interested in the whole stack, including the java part, there is a 'magic' object injected in the catch scope, so when you are in a catch block you can reference it by the name __exception__. This is a reference to the JavaScriptException <http://rhino/src/org/mozilla/javascript/JavaScriptException.java> wrapper for the thrown Error and you can invoke methods on it as usual. I hope this helps to those of us in the same boat. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#109 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAf0a7anlOoQqlOMlk_sMoOlL1TlGx4Mks5svgIkgaJpZM4Acyut> .

[rbri]

@gbrail Hi Greg, do you see a reason why your apigee patch is not merged into this repo. For HtmlUnit i still have a slightly different version of marc's patch. I think we have to integrate this in some way.

[rbri]

@gbrail knock, knock

[gbrail]

Since it's been a long time, it looks like the thing we want to enable is:

let x = Error.new('Message');

to have a "stack" property populated.

It looks like a pretty small change from the very old PR to fix that. Is that what we want to fix?

It also sounds like we have other problems, including the weird behavior of only adding line numbers if the special flag is set. It seems like it might be time to get rid of that flag, but I think that's a different issue.

[rbri]

@gbrail i guess this is solved by your last NativeError rework

[gbrail]

I think that we finally took care of this one!