Closed Bug 1430022 Opened 4 years ago Closed 2 months ago

Copy command on cyclic object should display an error message in the console

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(firefox59 wontfix, firefox92 fixed)

RESOLVED FIXED
92 Branch
Tracking Status
firefox59 --- wontfix
firefox92 --- fixed

People

(Reporter: nchevobbe, Assigned: claubatista)

Details

Attachments

(3 files, 1 obsolete file)

Steps to reproduce:
1. Open the webconsole
2. Evaluate the following code : `var a = {};a.b = a; copy(a);`


Expected results:

An error is displayed in the console, as if I was doing `var a = {};a.b = a; JSON.stringify(a);` ("TypeError: cyclic object value[Learn More]")

Actual results:

There is nothing displayed in the console, and when I try to paste the result of the copy command I get "/* TypeError: cyclic object value */".

This is not useful because : 
- I don't have immediate feedback (and I may already have switched to my editor to paste the thing I wanted to copy, and thus I have to go back to the console, and figure out what is going on)
- I don't have the "Learn more" link that could tell me what this error means.
Good idea, showing an error in the console would be much better than putting this on the clipboard
Priority: -- → P3
The patch from Comment 3 is already better than the current situation, but: 
- The error does not say what caused it. We should have something that say: "Error while copying object: {the actual exception}".
- The error is not l10n'ed
- There is no "learn more" link to make sense of the error.

Also, the error is not shown if you do the copy via the context menu entry (right click on object, and select "Copy object").

We should fire the same message as if you were doing `x = {}; x.y = x;JSON.stringify(x)`, only with a prefix to indicate it is a result of the copy command.
Product: Firefox → DevTools

@Nicholas can you please assign me this task, I am working on it

sure! thanks

Assignee: nobody → abhinav.23.april
Status: NEW → ASSIGNED

Bug 1430022 -Added error message in copy command on cyclic object value

Attached file Bug 1430022: Removed try catch block (obsolete) —

@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit

(In reply to Abhinav Raj from comment #10)

@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit

Please can you abandon the extra revision? If you visit it at https://phabricator.services.mozilla.com/D62046, the scroll down to the bottom. Just above the comments box, there's an Add Action option... select Abandon revision from that, then hit submit.

(In reply to Mark Banner (:standard8) from comment #11)

(In reply to Abhinav Raj from comment #10)

@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit

Please can you abandon the extra revision? If you visit it at https://phabricator.services.mozilla.com/D62046, the scroll down to the bottom. Just above the comments box, there's an Add Action option... select Abandon revision from that, then hit submit.

Done

(In reply to Abhinav Raj from comment #12)

(In reply to Mark Banner (:standard8) from comment #11)

(In reply to Abhinav Raj from comment #10)

@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit

Please can you abandon the extra revision? If you visit it at https://phabricator.services.mozilla.com/D62046, the scroll down to the bottom. Just above the comments box, there's an Add Action option... select Abandon revision from that, then hit submit.

Done

Hmm, I don't see any changes, it still looks like it is open. Did you hit submit after selecting abandon revision?

Flags: needinfo?(nchevobbe)
Flags: needinfo?(nchevobbe) → needinfo?(abhinav.23.april)
Attachment #9125079 - Attachment is obsolete: true

That's got it, thank you!

Flags: needinfo?(abhinav.23.april)

Abhinav, are you still working on this?

Flags: needinfo?(abhinav.23.april)

@nchevobbe I can take a look into this bug.

Thanks Alex, the bug is now yours :)

What needs to be done:

it would be nice to still pass the exception message to the client, and try to display it in the error message, but let's focus on having a message displayed first :)

Assignee: abhinav.23.april → alexsmp
Assignee: alexsmp → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(abhinav.23.april)

Hi! I'd like to work on this. :D

thanks for helping us Claudia, the bug is now yours :)

Assignee: nobody → contatodaclau
Status: NEW → ASSIGNED
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/55ab12720df1
[devtools] Adds an error message in the copy command on a cyclic object r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
You need to log in before you can comment on or make changes to this bug.