Closed Bug 1473418 Opened 6 years ago Closed 6 years ago

Toolbox.open makes some exceptions almost silent

Categories

(DevTools :: General, defect, P3)

Product:
DevTools
Component:
General
Type:
defect

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1473418 - Improve exception message occuring during toolbox opening.
59 bytes, text/x-review-board-request
jryans
: review+
Details 
While working on bug 1471853 and hacking the loader, I had an exception made almost silent.
I was having this output:
  console.error: ({})
  JavaScript error: resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/devtools.js, line 522: TypeError: toolId is undefined

The first one, console.error is related to the original exception,
while the second one is only an artifact of the first that doesn't help figuring out the original one.

This console.error relates to this line:
  https://searchfox.org/mozilla-central/rev/6ef785903fee6c0b16a1eab79d722373d940fd78/devtools/client/framework/toolbox.js#579
The console codebase poorly translate the loader exception, may be because that an old style NS_ERROR_FAILURE and not a JS exception...

With the upcoming patch, the exception is now printed like this:
console.error: "Exception while opening the toolbox" "[Exception... \"Component is not available\"  nsresult: \"0x80040111 (NS_ERROR_NOT_AVAILABLE)\"  location: \"JS frame :: resource://devtools/shared/base-loader.js :: Loader :: line 657\"  data: no]" "Loader@resource://devtools/shared/base-loader.js:657:9\nBrowserLoaderBuilder@resource://devtools/client/shared/browser-loader.js:175:17\nBrowserLoader@resource://devtools/client/shared/browser-loader.js:73:32\nopen/<@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/toolbox.js:438:29\nAsync*open@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/toolbox.js:437:19\ncreateToolbox@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/devtools.js:555:11\nasync*showToolbox@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/devtools.js:474:30\nasync*toggleToolboxCommand@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/devtools-browser.js:72:7\nonKeyShortcut@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/devtools-browser.js:246:9\nonKey@file:///mnt/desktop/gecko-dev/obj-firefox-artifact/dist/bin/browser/components/devtools-startup.js:601:7\nhookKeyShortcuts/xulKey<@file:///mnt/desktop/gecko-dev/obj-firefox-artifact/dist/bin/browser/components/devtools-startup.js:578:53\nEventListener.handleEvent*createKey@file:///mnt/desktop/gecko-dev/obj-firefox-artifact/dist/bin/browser/components/devtools-startup.js:625:5\nhookKeyShortcuts@file:///mnt/desktop/gecko-dev/obj-firefox-artifact/dist/bin/browser/components/devtools-startup.js:578:22\nhookWindow@file:///mnt/desktop/gecko-dev/obj-firefox-artifact/dist/bin/browser/components/devtools-startup.js:350:5\nonWindowReady@file:///mnt/desktop/gecko-dev/obj-firefox-artifact/dist/bin/browser/components/devtools-startup.js:293:5\n_delayedStartup@chrome://browser/content/browser.js:1583:5\nEventListener.handleEvent*onLoad@chrome://browser/content/browser.js:1382:5\nonload@chrome://browser/content/browser.xul:1:1\n"
JavaScript error: resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/devtools.js, line 522: TypeError: toolId is undefined
Assignee: nobody → poirot.alex
Priority: -- → P3
Comment on attachment 8989874 [details]
Bug 1473418 - Improve exception message occuring during toolbox opening.

https://reviewboard.mozilla.org/r/254872/#review261954

Seems like an improvement already, but does the console pretty printing for stacks work?  For example, it should show each frame on a separate line.  Unclear from your example if it does work for this...
Attachment #8989874 - Flags: review?(jryans) → review+
Comment on attachment 8989874 [details]
Bug 1473418 - Improve exception message occuring during toolbox opening.

https://reviewboard.mozilla.org/r/254872/#review261986
Comment on attachment 8989874 [details]
Bug 1473418 - Improve exception message occuring during toolbox opening.

https://reviewboard.mozilla.org/r/254872/#review261954

No, It prints as I pasted on bugzilla. I don't know if the console is expected to be featureful when it prints to the stdout? I haven't verified but I'm expecting it to be smart in the browser console.
On stdout, tt seems to stringify the stack and consider it as such. The new lines are printed as "\n"...
Comment on attachment 8989874 [details]
Bug 1473418 - Improve exception message occuring during toolbox opening.

https://reviewboard.mozilla.org/r/254872/#review261954

Ah right, I was talking about how it appears in Browser Console.  Might be worth double checking...
(In reply to J. Ryan Stinnett [:jryans] (use ni?) (on PTO until July 12) from comment #6)
> Comment on attachment 8989874 [details]
> Bug 1473418 - Improve exception message occuring during toolbox opening.
> 
> https://reviewboard.mozilla.org/r/254872/#review261954
> 
> Ah right, I was talking about how it appears in Browser Console.  Might be
> worth double checking...

Good call!
While it behaves better by passing e.stack for stdout usecase, the stack appears as a string on the browser console.
I switched to pass `e`, the exception as-is to console.error to keep a better behavior for the browser console
and I added a dump(e.stack + "\n") to tweak stdout behavior.
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/38b811b6c5e5
Improve exception message occuring during toolbox opening. r=jryans
https://hg.mozilla.org/mozilla-central/rev/38b811b6c5e5
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: