Last Comment Bug 549604 - Add (non ECMA) property "stack" to NativeError
: Add (non ECMA) property "stack" to NativeError
Status: RESOLVED FIXED
:
Product: Rhino
Classification: Components
Component: Core (show other bugs)
: other
: x86 Linux
: -- enhancement (vote)
: ---
Assigned To: Hannes Wallnoefer
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-02 05:40 PST by Marc
Modified: 2010-03-31 08:15 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch with unit test implementing NativeError's stack property and improving RhinoException.getScriptStacktrace in compiled mode (9.14 KB, patch)
2010-03-02 05:44 PST, Marc
no flags Details | Diff | Splinter Review
new patch (28.46 KB, patch)
2010-03-31 05:26 PDT, Hannes Wallnoefer
no flags Details | Diff | Splinter Review
slightly revised version of the second patch (27.19 KB, patch)
2010-03-31 06:26 PDT, Hannes Wallnoefer
no flags Details | Diff | Splinter Review

Description Marc 2010-03-02 05:40:41 PST
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
Comment 1 Marc 2010-03-02 05:44:51 PST
Created attachment 429708 [details] [diff] [review]
Patch with unit test implementing NativeError's stack property and improving RhinoException.getScriptStacktrace in compiled mode
Comment 2 Hannes Wallnoefer 2010-03-30 04:11:58 PDT
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.
Comment 3 Marc 2010-03-30 05:10:31 PDT
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.
Comment 4 Hannes Wallnoefer 2010-03-31 05:26:20 PDT
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.
Comment 5 Hannes Wallnoefer 2010-03-31 06:26:56 PDT
Created attachment 436176 [details] [diff] [review]
slightly revised version of the second patch
Comment 6 Hannes Wallnoefer 2010-03-31 08:15:07 PDT
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

Note You need to log in before you can comment on or make changes to this bug.