Netmonitor JS stack traces should show the frame's asyncCause

RESOLVED FIXED in Firefox 50

Status

DevTools
Netmonitor
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: jsnajdr, Assigned: jsnajdr)

Tracking

Trunk
Firefox 50

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
If a stack frame has an asyncCause field, show it.
(Assignee)

Updated

2 years ago
Blocks: 1134073
(Assignee)

Comment 3

2 years ago
Created attachment 8762999 [details] [diff] [review]
Netmonitor JS stack traces should show the frame's asyncCause

Backend:
added "asyncCause" field to the saved stack frames

Frontend:
display the asyncCause in the stack trace. I'm displaying it as an extra row between the two frames that are connected by the async action. For example:

setTimeoutHandler script.js:10
(Async: setTimeout handler)
functionThatCallsSetTimeout script.js:11

It's much easier to understand than what is currently used by console.trace:

setTimeoutHandler script.js:10
(Async: setTimeout handler) functionThatCallsSetTimeout script.js:11

Also, Chrome displays async frames this way in their debugger.

I'll also update the console.trace output if this approach is liked.

Tests:
added some async requests to browser_net_cause.js and checking the stack frames there. Try run is green.
Attachment #8762999 - Flags: review?(poirot.alex)
(Assignee)

Updated

2 years ago
Assignee: nobody → jsnajdr
Comment on attachment 8762999 [details] [diff] [review]
Netmonitor JS stack traces should show the frame's asyncCause

Review of attachment 8762999 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks for polishing the stacks!
Sorry for the review latency.

::: devtools/client/netmonitor/netmonitor-view.js
@@ +2347,5 @@
> +        // if there is asyncCause, append a "divider" row into the trace
> +        let asyncFrameEl = doc.createElementNS(HTML_NS, "div");
> +        asyncFrameEl.className = "stack-frame stack-frame-async";
> +        asyncFrameEl.textContent =
> +          WEBCONSOLE_L10N.getFormatStr("stacktrace.asyncStack", asyncCause);

I'm wondering if this "Async: " is clear enough.
It doesn't feel 100% obvious to me but I don't have alternatives to suggest.

It would be great to better understand there is multiple run-to-completion stacks. I think your approach to put this async message on a new line make sense here and should helper understand that!

::: devtools/client/netmonitor/test/browser_net_cause.js
@@ +72,5 @@
>      method: "POST",
>      url: EXAMPLE_URL + "beacon_request",
>      causeType: "beacon",
>      causeUri: CAUSE_URL,
> +    stack: [{ fn: "performBeaconRequest", file: CAUSE_FILE_NAME, line: 30 }]

Isn't beacon also going to have setTimeout async cause?
Attachment #8762999 - Flags: review?(poirot.alex) → review+
(Assignee)

Comment 5

2 years ago
(In reply to Alexandre Poirot [:ochameau] from comment #4)
> Isn't beacon also going to have setTimeout async cause?

Yes, it is, but I'm not really testing the sendBeacon's async stack - I'm calling it async only because I have to - otherwise, the requests don't have a reliable order. Sometimes the netmonitor shows the beacon before the first async XHR request, sometimes it's at the end.
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/db5573bc588d
Netmonitor JS stack traces should show the frame's asyncCause r=ochameau
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/db5573bc588d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.