1007656 - Support formatting the current stack using Log.jsm / Log.stackTrace()

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[:Irving Reid (No longer working on Firefox)]

The Log.stackTrace() function in Log.jsm requires an exception object (either a JS Error or an XPCOM nsIException). It would be useful to be able to format a stack trace directly (e.g. Log.stackTrace(Components.stack)), and even more useful if calling it with no arguments formatted the current stack.

Suggested API:

Log.stackTrace(Error),
Log.stackTrace(nsIException):
  - format the stack trace contained in the exception (existing behaviour)
Log.stackTrace(nsIStackFrame):
  - format the trace as given
Log.stackTrace():
  - format Components.stack.caller as an nsIStackFrame

There aren't very many callers of the existing Log.stackTrace(error) API so we could cut that part of the API and have those callers pass the stack frame directly, but I'm not sure it's worth the work.

Comment 1

[Mike Hoye [:mhoye]]

Upgrading this to [mentored bug] from good first;

Comment 2

[Manu Jain]

I would like to patch this bug. Can you assign this to me? But I need some help in going forward. Thanks!!

Comment 3

[:Irving Reid (No longer working on Firefox)]

Thanks for looking at this. As the bug description says, we currently have an API in toolkit/modules/Log.jsm that gives us a formatted stack trace when called with a JavaScript error object, function stackTrace() at around http://dxr.mozilla.org/mozilla-central/source/toolkit/modules/Log.jsm?#164

This function uses what we sometimes call "duck typing" to figure out what kind of object is passed in as its parameter, in order to correctly format the JS stack trace for that object. The current implementation recognizes JavaScript 'Error' objects, which are exceptions typically generated by JS code, and 'nsIException' objects, which are Mozilla-specific exception objects generated by our XPCOM module system (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIException).

We'd like to add some more cases to the logic in that function, to handle the cases:

 - if the parameter 'e' is an nsIStackFrame object (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIStackFrame), format the stack it represents.
 - if no value was passed for 'e', use the built-in (Mozilla internal) value 'Components.stack.caller' (which is an nsIStackFrame object).

Conveniently, all the formatting code you need is already in that function, because the 'e.location' value (http://dxr.mozilla.org/mozilla-central/source/toolkit/modules/Log.jsm?#167) is an nsIStackFrame.

You can test the type of an object (in this case) by using the JS 'instanceof' operator:

if (e instanceof Ci.nsIStackFrame) {
 ...
}

(Ci is a shortcut definition that we commonly use in Mozilla modules, defined at line 9 of Log.jsm. The actual name of the object interface is 'Components.interfaces.nsIStackFrame')


For this patch, you should also include some test cases in the file toolkit/modules/tests/xpcshell/test_Log.js; if you want to practice test-driven development, feel free to write the tests first ;-)

You can see from that file that the wrapper around each test looks like:

add_task(function* invent_a_meaningful_test_name() {
  ... code to run the test here
});

and you can run all the tests in this file using the command

./mach xpcshell-test test_Log.js


Full documentation for this test harness is at https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests

If you have any questions, you can ask here or on IRC at irc.mozilla.org in the #introduction channel (see https://wiki.mozilla.org/IRC)

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 5

[sayuree]

@markh As far as I understood, we need to consider 2 cases:
Case 1:
Input: 'e' is an nsIStackFrame object
Output: format the stack it represents? What should be the output in this case? How should it be formatted?
Also the information about nsiStackFrame is no longer available via https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIStackFrame

Case 2:
Input: 'e' has no value (empty)
Output: 'Components.stack.caller' (which returns nsiStackFrame object)

Is it right? Can you please give some direction?

Comment 6

[Mark Hammond [:markh] [:mhammond]]

I'll do my best, but lots has changed in the last 8 years :)

nsIStackFrame is defined in https://searchfox.org/mozilla-central/source/xpcom/base/nsIException.idl - notably, it has a formattedStack attribute, which is probably all you need to do - there's no value in trying to invent our own formatting here.

Components.stack.caller does still exist this link shows all existing references to it. So case 2 is, pretty much, the same as case 1, but using Components.stack.caller instead of the arg.

Specifically, in the browser console you can see something like:

>> Components.stack.caller.formattedStack
"getEvalResult@resource://devtools/server/actors/webconsole/eval-with-debugger.js:243:24
exports.evalWithDebugger@resource://devtools/server/actors/webconsole/eval-with-debugger.js:167:14
evaluateJS@resource://devtools/server/actors/webconsole.js:1118:38
evaluateJSAsync/<@resource://devtools/server/actors/webconsole.js:1010:35
exports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:103:22
"

which looks fine, although you probably want to trim trailing \n chars.

So adding support for these cases here is probably what you want to do - in other words, we would only return the new stack when we'd otherwise be returning "No traceback available"

The other part would be tests, and as usual, these are often the most painful. There are a couple of "test_Log*" files in this directory which might be a good starting point - but I guess you probably want a new file just for this feature, based on test_Log_stackTrace.js, but named something like test_Log_nsIStackFrame.js or similar. For the test, it's probably OK to test 2 things - one when Components.stack.caller is explicitly passed for your case 1, and one when nothing is passed (your case 2) - which would still be using Components.stack.caller inside the module as described.

Comment 7

[sayuree]

Currently, there is a problem with the launch of a browser console, followed by this error:
JavaScript error: resource://devtools/shared/loader/base-loader.js, line 169: Error: Module `devtools/shared/webconsole/messages` is not found at resource://devtools/shared/webconsole/messages.js

Comment 8

[Mark Hammond [:markh] [:mhammond]]

Sorry, I've no idea what could cause that - maybe your build isn't complete?

Comment 9

[sayuree]

Attachment: Bug 1007656 - Have Log.jsm handle nsIStackFrame instances. r=markh

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77f99cc6d00f
Have Log.jsm handle nsIStackFrame instances. r=markh

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/77f99cc6d00f