Add (non ECMA) property "stack" to NativeError

RESOLVED FIXED

Status

Rhino
Core
--
enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Marc, Assigned: Hannes Wallnoefer)

Tracking

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.18) Gecko/2010021501 Ubuntu/9.04 (jaunty) Firefox/3.0.18
Build Identifier: 

NativeError already has two interesting non ECMA properties: lineNumber and fileName. It would be useful to have property stack as well as done in different browsers.

Additionally, the information returned by RhinoException.getScriptStackTrace vary depending on the optimization level used. In interpreted mode in contains the function name whereas it is not the case in compiled mode.

Reproducible: Always
(Reporter)

Comment 1

8 years ago
Created attachment 429708 [details] [diff] [review]
Patch with unit test implementing NativeError's stack property and improving RhinoException.getScriptStacktrace in compiled mode
(Assignee)

Updated

8 years ago
Assignee: nobody → hannes
(Assignee)

Comment 2

8 years ago
Thanks for report and patch. I guess mirroring the stack format from Firefox is important to you? I prefer the one we have in RhinoException.getScriptStackTrace() for readability, but I think anything that's easy to parse is ok.

One thing I'd like to add to your patch is including support for the stack property for Errors generated via new Error() when the Context.FEATURE_LOCATION_INFORMATION_IN_ERROR feature is enabled (see bug #342807), so I consider doing a bit more refactoring on that.
(Reporter)

Comment 3

8 years ago
Indeed it is important for me to mirror Firefox format. RhinoException.getScriptStackTrace is a bit more coherent with this patch but if you really want to improve the situation in this area, I think that RhinoException should rather provide access to a StackTraceElement[]. It would be far better than having to parse strings.

You're right, it makes sense to handle Context.FEATURE_LOCATION_INFORMATION_IN_ERROR too.
(Assignee)

Comment 4

8 years ago
Created attachment 436164 [details] [diff] [review]
new patch

Rewritten patch. This adds getScriptStack() methods to RhinoExceptions to get the exception's stack as array of the (newly introduced) ScriptStackElement class. The default stack format is the "traditional" Rhino format, but there's a static useMozillaStackStyle(boolean) method in RhinoException to switch to Mozilla/Firefox stack format. Is that a viable solution for you, Marc?

The patch also sets the stack on Errors thrown by script code if the FEATURE_LOCATION_INFORMATION_IN_ERROR context feature is enabled.
(Assignee)

Comment 5

8 years ago
Created attachment 436176 [details] [diff] [review]
slightly revised version of the second patch
Attachment #429708 - Attachment is obsolete: true
Attachment #436164 - Attachment is obsolete: true
(Assignee)

Comment 6

8 years ago
Committed the last patch. Let me know if you find any problems with it.

Checking in src/org/mozilla/javascript/Interpreter.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/Interpreter.java,v  <--  Interpreter.java
new revision: 1.361; previous revision: 1.360
done
Checking in src/org/mozilla/javascript/JavaScriptException.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/JavaScriptException.java,v  <--  JavaScriptException.java
new revision: 1.28; previous revision: 1.27
done
Checking in src/org/mozilla/javascript/NativeError.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/NativeError.java,v  <--  NativeError.java
new revision: 1.52; previous revision: 1.51
done
Checking in src/org/mozilla/javascript/RhinoException.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/RhinoException.java,v  <--  RhinoException.java
new revision: 1.14; previous revision: 1.13
done
Checking in src/org/mozilla/javascript/ScriptRuntime.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptRuntime.java,v  <--  ScriptRuntime.java
new revision: 1.327; previous revision: 1.326
done
Checking in src/org/mozilla/javascript/ScriptStackElement.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptStackElement.java,v  <--  ScriptStackElement.java
new revision: 1.3; previous revision: 1.2
done
RCS file: /cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/ErrorPropertiesTest.java,v
done
Checking in testsrc/org/mozilla/javascript/tests/ErrorPropertiesTest.java;
/cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/ErrorPropertiesTest.java,v  <--  ErrorPropertiesTest.java
initial revision: 1.1
done
Checking in testsrc/org/mozilla/javascript/tests/StackTraceTest.java;
/cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/StackTraceTest.java,v  <--  StackTraceTest.java
new revision: 1.2; previous revision: 1.1
done
Checking in testsrc/org/mozilla/javascript/tests/Utils.java;
/cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/Utils.java,v  <--  Utils.java
new revision: 1.3; previous revision: 1.2
done
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.