1822488 - console.error doesn't expose stack for original error passed from try...catch block

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[:Gijs (out for now; he/him)]

What were you doing?

0. nightly from March 14, 2023
1. open browser console
2. Open Firefox > App Menu > History > Manage History. (or cmd-shift-o / ctrl-shift-o)
3. Click on Import data and Backpup > Import Data From Another Browser.
4. Observe the browser console.

What happened?

The browser console shows an error, and if you click the disclosure arrow the following stack is shown:

DOMException: Could not parse path (null): NS_ERROR_FILE_UNRECOGNIZED_PATH MigratorBase.sys.mjs:494:15
    isSourceAvailable resource:///modules/MigratorBase.sys.mjs:494
    spinResolve resource:///modules/MigrationUtils.sys.mjs:319
    spinResolve chrome://browser/content/migration/migration.js:166
    onImportSourcePageShow chrome://browser/content/migration/migration.js:200
    init chrome://browser/content/migration/migration.js:148
    onload chrome://browser/content/migration/migration.xhtml:1

but this points to the console.error call.

What should have happened?

The disclosure arrow should show the stack for the error/exception that caused us to enter the catch block.

If I substitute Cu.reportError(ex) (and add an // eslint-disable-next-line comment to prevent it from being autofixed back to console.error!) then I see the expected behaviour.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Looks like we're not showing trace for Exceptions (but we do for Errors)
Alex worked on something similar not long ago, he'll provide some information

This should block the removal of Cu.reportError

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1822488 - [devtools] Display stacktraces for DOMException's.

We are currently showing stacks for Error-like objects but not for DOMExceptions.
console.log("foo", new Error("bar")) would print bar's stacktraces, while we wouldn't for: console.log("foo", new DOMException("bar")).

Should we merge DOMException and Error previewers?
DOMException is only having code and result attributes which Error objects don't have.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1822488 - [devtools] Display stacks when logging Component.Exception objects.

We are current displaying stacks for Error but not for Component.Exception objects.

Comment 4

[Alexandre Poirot [:ochameau]]

This type of bugs may explain why we refrain from using console API... Thanks for reporting these issues.

In the submitted patches, I'm not changing the stack being displayed when clicking the left arrow as you may still be interested to know where the messages are coming from.
In addition to that, you may pass an arbitrary number of objects to console.error and would like to see the stack of all arguments:
console.error("Something went wrong", exception1, exception2)
(Even if that sounds very unlikely to log two distinct exceptions...)
Last but not least, we would be consistant with the existing behavior of console.error("something went wront", new Error("foo")).
Both Chrome and Firefox are currently showing the console.error callsite in the arrow expand and are both displaying the Error exception right away in the console output.

Mark, could you confirm that the second patch address the issue you raised February 13th on Element #devtools?

I'd like to propose that we have better console output for errors raised via Components.Exception objects. Is devtools the right place to handle that, or should it be somewhere else in JavaScript/xpconnect land?

Comment 5

[Mark Banner (:standard8)]

Attachment: Before (left) & After (right)

(In reply to Alexandre Poirot [:ochameau] from comment #4)

Mark, could you confirm that the second patch address the issue you raised February 13th on Element #devtools?

I'd like to propose that we have better console output for errors raised via Components.Exception objects. Is devtools the right place to handle that, or should it be somewhere else in JavaScript/xpconnect land?

I applied both patches and did some tests and I think it looks a lot better and is much easier to read - attachment as an example.

You do loose the Exception note at the start of the message, but I don't think that actually matters.

Thank you for working on this, I think it'll make it much easier for people who come across these.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55d0262ce61c
[devtools] Display stacktraces for DOMException's. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/af69bb79c76b
[devtools] Display stacks when logging Component.Exception objects. r=devtools-reviewers,nchevobbe

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/55d0262ce61c
https://hg.mozilla.org/mozilla-central/rev/af69bb79c76b