Closed Bug 1822488 Opened 1 year ago Closed 1 year ago

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

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(firefox113 fixed)

RESOLVED FIXED
113 Branch
Tracking Status
firefox113 --- fixed

People

(Reporter: Gijs, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

What were you doing?

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

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

Severity: -- → S3
Flags: needinfo?(poirot.alex)
Priority: -- → P3

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.

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

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

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?

Flags: needinfo?(poirot.alex) → needinfo?(standard8)

(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.

Flags: needinfo?(standard8)
Pushed by apoirot@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: