Closed Bug 1007656 Opened 11 years ago Closed 2 years ago

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

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: Irving, Assigned: sabina.zaripova, Mentored)

References

Details

(Whiteboard: [lang=js])

Attachments

(1 file)

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.
Whiteboard: [mentor=irving][lang=js] → [mentor=irving][lang=js][good first bug]
Upgrading this to [mentored bug] from good first;
Whiteboard: [mentor=irving][lang=js][good first bug] → [mentor=irving][lang=js][mentored bug]
Mentor: irving
Whiteboard: [mentor=irving][lang=js][mentored bug] → [lang=js][mentored bug]
Whiteboard: [lang=js][mentored bug] → [lang=js]
I would like to patch this bug. Can you assign this to me? But I need some help in going forward. Thanks!!
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)
Assignee: nobody → manu.jain13
Status: NEW → ASSIGNED
Mentor: irving
Mentor: mhammond

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

Assignee: manu.jain13 → nobody
Status: ASSIGNED → NEW

@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?

Flags: needinfo?(markh)

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.

Flags: needinfo?(markh)

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

Flags: needinfo?(markh)

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

Flags: needinfo?(markh)
Assignee: nobody → sabina.zaripova
Status: NEW → ASSIGNED
Attachment #9276557 - Attachment description: Bug 1007656 - add cases to stackTrace. r=markh → Bug 1007656 - Have Log.jsm handle nsIStackFrame instances. r=markh
Pushed by mhammond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/77f99cc6d00f Have Log.jsm handle nsIStackFrame instances. r=markh
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: