1428520 - Repeat count does not appear for uncaught errors

Status: RESOLVED DUPLICATE of bug 1405245

(DevTools :: Console, defect, P2)

Description

[Matt McCutchen]

Attachment: PageError: take repeat from props rather than message

I noticed that the message repeat count in the developer console does not work for uncaught errors ("page errors"), while it does work for messages logged with the console API.  For example, run the following in the developer console:

for (let x = 0; x < 3; x++) window.setTimeout(() => { console.error("test"); }, 0);
for (let x = 0; x < 3; x++) window.setTimeout(() => { throw new Error("test"); }, 0);

The first line gives a nice "3 repeats" marker, while the second gives one log message (so apparently the duplication is detected) but without the marker.

Reproduced on Firefox 57.0.1 and Nightly (build ID 20180105220204).

After looking at https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/new-console-output/components/message-types/ConsoleApiCall.js and https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/new-console-output/components/message-types/PageError.js, I think the attached patch will fix the problem.  Monkeypatching the code using a conditional breakpoint in the browser toolbox appears to confirm my understanding, though I haven't tried actually rebuilding the browser.

Comment 1

[Matt McCutchen]

Attachment: PageError: take repeat from props rather than message (corrected file path)

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Hello Matt, thanks for filing this issue AND providing a fix :)
Do you want to work on this further ? We could use a test to make sure the patch does fix the issue.
Something like : 

diff --git a/devtools/client/webconsole/new-console-output/test/components/page-error.test.js b/devtools/client/webconsole
--- a/devtools/client/webconsole/new-console-output/test/components/page-error.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/page-error.test.js
@@ -269,4 +269,15 @@ describe("PageError component:", () => {
     expect(locationLink.length).toBe(1);
     expect(locationLink.text()).toBe("test-console-api.html:2:6");
   });
+
+  it("displays the repeat bubble", () => {
+    const message = stubPreparedMessages.get("ReferenceError: asdf is not defined");
+    const wrapper = render(PageError({
+      message,
+      serviceContainer,
+      timestampsVisible: true,
+      repeat: 3,
+    }));
+    expect(wrapper.find(".message-repeats").text()).toBe("3");
+  });
 });

Without your patch, it fails. You can run the tests by doing `cd devtools/client/webconsole && yarn && yarn test`
You may need to run `cd devtools/client/netmonitor && yarn` before.

If you don't have time, no worries, I'll take your patch and add the test on top of it.

Thanks again !

Comment 3

[Pradeep Gangwar [:pradeepgangwar]]

I am interested to work on this.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Hello Pradeep, thanks for your help !
I assign you the bug. I think you can take Matt's patch and add a test similar to the one I'm talking to in Comment 2.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Hello Pradeep,
Do you need any help on this ?

Comment 6

[Pradeep Gangwar [:pradeepgangwar]]

Sorry, I was busy. I will try resolving it this weekend.

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

no need to apologies :) I just wanted to make sure you weren't blocked by something.

Comment 8

[Pradeep Gangwar [:pradeepgangwar]]

Hi, I looked into this and found that bug is fixed. Looks like this commit https://github.com/mozilla/gecko-dev/commit/3429eb9b3f9270a5fc200f42c2ab2afe13cff90d fixed the bug. Let me know if it still requires more work. :)

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Hello,

Yes, sorry about that but we had bug that was relying on this to be fixed, so we did it :)