Closed Bug 1566780 Opened 1 year ago Closed 7 months ago

Have option to hide WS control frames

Categories

(DevTools :: Netmonitor, enhancement, P3)

enhancement

Tracking

(firefox76 fixed)

RESOLVED FIXED
Firefox 76
Tracking Status
firefox76 --- fixed

People

(Reporter: christoph-wa, Assigned: kishlaya.j, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(2 files)

There should be an option in the toolbar or in a context menu which gives me the ability to hide WS control frames.

There are three control opcodes specified https://tools.ietf.org/html/rfc6455#section-5.2

  • ping 0x9
  • pong 0xa
  • close 0x8

frames having one of these opcodes are handled directly by the WS engine and are not delivered to the Webapp.

Chrome doesn't show control frames

It looks from old docs that Chrome showed color coded control frames before, I have not checked when they stopped.
Deprecated docs: https://developers.google.com/web/tools/chrome-devtools/network/resource-loading#view_websocket_frames . Latest docs, still mentioning color-coded opcodes: https://developers.google.com/web/tools/chrome-devtools/network/reference#frames . Having a ping/pong every multiple times per minute does seem way too noisy and obfuscates the actual user-triggered messages.

Seeing the heartbeat of ping and pong as one entry, maybe with latency seems actually kinda useful.

This might be actually handled by the filter toolbar, but the non-dropdown version would have worked better.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3

Let's add this as a new filter to the toolbar for now. This might be a good first bug with UI and React/Redux work.

Default should be filtering control frames for users, as most should not care about the heartbeat of their servers.

Keywords: good-first-bug

thanks for coming back to this! cool

Default should be filtering control frames ..

yep, that'd be perfect!


unrelated to this issue here: skimming thru the docs you linked above I am stumbling on

"Analyze the frames of a WebSocket Connection"

Frames? Is it really websocket frames, rather than websocket messages?

  • On the wire, websocket transports data and control frames only.
  • Data frames are reassembled in FF under the hood before providing the complete payload of a websocket messages (reassembled from the individual data frame payloads)
  • There are only control frames, no control messages - iow, there is no fragmentation with control traffic
  • The API for JS (the WHATWG WebSocket API) only has data messages - no data frame level access, no control frame access or control

Now, I think the websocket subprotocol parsing code (eg for WAMP or others) should be fired with the websocket message, not frame to parse, because it would need to do websocket frame reassembly otherwise.

If that is so, I am also wondering about the naming of

getFramePayload(selectedFrame.payload, ...

further, if the inspector would normally be showing data messages (and parsed content for those) only, and showing control frames would be a further option to activate by the user, then the inspector would need to show a mix of websocket data message (parsed) and websocket control frames (unparsed payload, control frame type only shown), while the parsing callback (to parse the websocket subprotocol) would only be fired with whole data messages (because subprotocol specific control frames are not allowed with websocket - only websocket extensions can use custom control frames).

sorry for this long OT mumblings, but this got me confused and wondering .. maybe just me;)

sorry for this long OT mumblings, but this got me confused and wondering .. maybe just me;)

Consistent naming between messages and frames has been a challenge. The table shows frame data as well (opCode) while details panel is all about the message.

the inspector would need to show a mix of websocket data message (parsed) and websocket control frames (unparsed payload, control frame type only shown), while the parsing callback (to parse the websocket subprotocol) would only be fired with whole data messages (because subprotocol specific control frames are not allowed with websocket - only websocket extensions can use custom control frames).

This is good to keep in mind for bug 1621822, to not expect to show any data for control frames but maybe just the opCode.

(In reply to :Harald Kirschner :digitarald from comment #2)

Let's add this as a new filter to the toolbar for now. This might be a good first bug with UI and React/Redux work.

We currently have the following filters:

  • All
  • Sent
  • Received

What the new filter should be?

Also, it looks like "no control frames" should rather be an option that can be used in combination with existing filters, no?

Honza

Flags: needinfo?(hkirschner)

Also, it looks like "no control frames" should rather be an option that can be used in combination with existing filters, no?

that would be nice!

because websocket PING control frame can be both sent (when the browser initiates a server ping-pong .. which I don't know if FF does) as well as received (when the server initiates a ping-pong). and consequently for the PONG control frame replies.

besides PING and PONG, there is CLOSE, which again can be initiated from both sides - and this is indeed of practical app developer interest! as in: "did my UI initiate a close or the server?"

besides those control frame types, I am not aware of any other deployed or practically relevant. websocket extensions can define additional control frames, but only websocket-mux did to my knowledge, and that never went live. permessage-deflate is the only websocket extension in the wild, and that doesn't define extra control frame types.

Also, it looks like "no control frames" should rather be an option that can be used in combination with existing filters, no?

That makes sense, the dropdown doesn't work.

We had prior UX explorations for the toolbar using the network panel filter badges ([All] [HTML] [CSS] …) vs dropdown, so part of this would be using that pattern probably. [Sent] [Received] [Control]. Are there more ideas around to explore?

Flags: needinfo?(hkirschner) → needinfo?(odvarko)
Attached image image.png

I like the idea, it would make the UI consistent.

I made a mockup. All the filter buttons [All] [Sent] [Received] [Control] take some horizontal space so, I think we should take the same approach as the Network panel (or Console panel) and split the Toolbar into two rows in case the side bar is thin. E.g. when the Filter Messages input box is less than 100px wide.

The other and simple way would be to turn the menu items (in the drop down) into checkboxes (like options menu) but it's obviously not that nice.

Honza

Flags: needinfo?(odvarko) → needinfo?(hkirschner)

Hi I would like to work on this

Assignee: nobody → biboswan98
Mentor: odvarko
Status: NEW → ASSIGNED

I think we should take the same approach as the Network panel (or Console panel) and split the Toolbar into two rows in case the side bar is thin. E.g. when the Filter Messages input box is less than 100px wide.

Looks great and the responsive toolbar would help here as well. ni? victoria might have other options in mind?

All the filter buttons [All] [Sent] [Received] [Control] take some horizontal space so

We can remove [ All ] to save space.

Flags: needinfo?(hkirschner) → needinfo?(victoria)

I tried working on this bug.

I didn't make changes to UI because I wanted to first get the logic right.
Above patch makes the following changes:
All displays all frames (except control frames)
Sent displays all sent frames (except sent control frames)
'Receiveddisplays all received frames (except received control frames)Control Frames` displays only the control frames

Can you please check if this is correct?

Also, right now the UI uses the Menu panel. Which panel should I replace it with?
Can you give me pointers to which files should I look at to change this UI?

Flags: needinfo?(odvarko)

I think it would be better if All, Sent, Received remain unchanged and Control is an additional filter.
For example, if you want to see only sent messages and control frames, check both Send and Control.
This additional filtering could be clarified with an extra visual space between these checkboxes

I think it would be better if All, Sent, Received remain unchanged and Control is an additional filter.

That could also work (type dropdown + control toggle filter).

(In reply to :Harald Kirschner :digitarald from comment #14)

I think it would be better if All, Sent, Received remain unchanged and Control is an additional filter.

That could also work (type dropdown + control toggle filter).

This sounds good to me too

:digitarald I have made the appropriate changes. Please check and let me know.

@Kishlaya: you are working on a bug that's already assigned to someone else.
@biboswan98: are you working on this?

(In reply to :Harald Kirschner :digitarald from comment #14)

That could also work (type dropdown + control toggle filter).

If multiple options can be selected in the Filter dropdown - the label should reflect that e.g. show

  • Received
  • Received + Control

Or the following:

  • All + Control

Which means that All isn't really everything (which might be a bit confusing)
I like what's suggested in Comment #7 and #8 and #10
I.e. having: [Sent] [Received] [Control] check-buttons in the toolbar (just like we have for the Network panel itself)

Honza

Flags: needinfo?(odvarko)

@Honza I am sorry, I didn't notice that. When I first visited this bug, it was unassigned and so I started looking into it. Meanwhile, I also saw there was another bug (1621822) with control frames. I was able to fix that first. Now when I returned to this, I didn't see that it had already been assigned.

(In reply to Kishlaya from comment #18)

@Honza I am sorry, I didn't notice that. When I first visited this bug, it was unassigned and so I started looking into it. Meanwhile, I also saw there was another bug (1621822) with control frames. I was able to fix that first. Now when I returned to this, I didn't see that it had already been assigned.

Sure, no problem and thanks for fixing bug 1621822 !

Honza

@biboswan98: please let me know if you want to work on this or whether Kishlaya can continue with his patch.

We were also discussing the UX today again and agreed on the following simple solution:

  • Let's add Control as an additional filter into the existing filter drop down.
  • The Control filter should be at the end of the drop down separated by horizontal line (a separator).
  • The Control filter should show a check icon when on
  • The label for the entire drop down doesn't indicate whether the Control filter is checked or not. This makes it a bit hidden but, that's ok since most of the people won't need it.

Honza

Flags: needinfo?(biboswan98)

Since Kishlaya has already progressed i think its better to let him continue

Flags: needinfo?(biboswan98)

Thanks for the update!
Honza

Assignee: biboswan98 → kishlaya.j
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05de83a88c61
Added another filter frame type for viewing control frames separately r=Honza

Backed out changeset 05de83a88c61 (bug 1566780) for devtools failures complaining about browser_net_ws-early-connection.js

Push with failures:https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C7%2Cdebug%2Cmochitests%2Ctest-windows7-32%2Fdebug-mochitest-devtools-chrome-e10s-2%2Cm%28dt2%29&fromchange=238a37af3863f840c68b00ad137eef6f39f49292&tochange=670a20bfd18a2ced1db8fc9a3234ef218c202b26&selectedJob=295340703

Backout link: https://hg.mozilla.org/integration/autoland/rev/670a20bfd18a2ced1db8fc9a3234ef218c202b26

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=295340703&repo=autoland&lineNumber=26775

...
[task 2020-03-30T11:14:40.336Z] 11:14:40     INFO - Entering test bound 
[task 2020-03-30T11:14:40.336Z] 11:14:40     INFO - Initializing a network monitor pane.
[task 2020-03-30T11:14:40.337Z] 11:14:40     INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html
[task 2020-03-30T11:14:40.338Z] 11:14:40     INFO - Tab added and finished loading
[task 2020-03-30T11:14:40.338Z] 11:14:40     INFO - Net tab added successfully: http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html
[task 2020-03-30T11:14:40.338Z] 11:14:40     INFO - Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)." {file: "chrome://global/content/elements/panel.js" line: 62}]
[task 2020-03-30T11:14:40.339Z] 11:14:40     INFO - Buffered messages logged at 11:13:11
[task 2020-03-30T11:14:40.340Z] 11:14:40     INFO - Network monitor pane shown successfully.
[task 2020-03-30T11:14:40.340Z] 11:14:40     INFO - Disabling cache and reloading page.
[task 2020-03-30T11:14:40.340Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 0/1, PayloadReady: 1/1EventTimings: 0/1, got NetMonitor:PayloadReady for server0.conn151.netEvent4
[task 2020-03-30T11:14:40.341Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 1/1, PayloadReady: 1/1EventTimings: 0/1, got NetMonitor:NetworkEvent for server0.conn151.netEvent4
[task 2020-03-30T11:14:40.341Z] 11:14:40     INFO - Got marker: dom-interactive
[task 2020-03-30T11:14:40.341Z] 11:14:40     INFO - Got marker: dom-complete
[task 2020-03-30T11:14:40.342Z] 11:14:40     INFO - Got two timeline markers, done waiting
[task 2020-03-30T11:14:40.342Z] 11:14:40     INFO - Console message: [JavaScript Warning: "Relative positioning of table rows and row groups is now supported. This site may need to be updated because it may depend on this feature having no effect." {file: "resource://devtools/client/netmonitor/src/components/request-list/RequestListHeader.js" line: 545}]
[task 2020-03-30T11:14:40.342Z] 11:14:40     INFO - Cache disabled when the current and all future toolboxes are open.
[task 2020-03-30T11:14:40.343Z] 11:14:40     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_ws-early-connection.js | Request to reconfigure the tab was recorded. - 
[task 2020-03-30T11:14:40.343Z] 11:14:40     INFO - Clearing requests in the console client.
[task 2020-03-30T11:14:40.344Z] 11:14:40     INFO - Clearing requests in the UI.
[task 2020-03-30T11:14:40.344Z] 11:14:40     INFO - Starting test... 
[task 2020-03-30T11:14:40.345Z] 11:14:40     INFO - Load document "http://mochi.test:8888/browser/devtools/client/netmonitor/test/html_ws-early-connection-page.html"
[task 2020-03-30T11:14:40.345Z] 11:14:40     INFO - Waiting for page to be loaded…
[task 2020-03-30T11:14:40.345Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 1/3, PayloadReady: 0/3EventTimings: 0/3, got NetMonitor:NetworkEvent for server0.conn151.netEvent23
[task 2020-03-30T11:14:40.346Z] 11:14:40     INFO - Buffered messages logged at 11:13:12
[task 2020-03-30T11:14:40.346Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 1/3, PayloadReady: 1/3EventTimings: 0/3, got NetMonitor:PayloadReady for server0.conn151.netEvent23
[task 2020-03-30T11:14:40.346Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 1/3, PayloadReady: 1/3EventTimings: 1/3, got NetMonitor:NetworkEventUpdated:EventTimings for server0.conn151.netEvent23
[task 2020-03-30T11:14:40.347Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 2/3, PayloadReady: 1/3EventTimings: 1/3, got NetMonitor:NetworkEvent for server0.conn151.netEvent42
[task 2020-03-30T11:14:40.347Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 3/3, PayloadReady: 1/3EventTimings: 1/3, got NetMonitor:NetworkEvent for server0.conn151.netEvent58
[task 2020-03-30T11:14:40.347Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 3/3, PayloadReady: 2/3EventTimings: 1/3, got NetMonitor:PayloadReady for server0.conn151.netEvent42
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 3/3, PayloadReady: 2/3EventTimings: 2/3, got NetMonitor:NetworkEventUpdated:EventTimings for server0.conn151.netEvent42
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - Buffered messages logged at 11:13:14
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - > Network event progress: NetworkEvent: 3/3, PayloadReady: 3/3EventTimings: 2/3, got NetMonitor:PayloadReady for server0.conn151.netEvent58
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - → page loaded
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - Waiting for netmonitor to be reloaded…
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - Waiting for netmonitor updates after page reload
[task 2020-03-30T11:14:40.348Z] 11:14:40     INFO - → panel reloaded
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - Waiting for target 'navigate' event…
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - → 'navigate' emitted
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_ws-early-connection.js | There should be three requests - 
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_ws-early-connection.js | There must be one WS connection request - 
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - Waiting for WS frames...
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - Buffered messages logged at 11:13:48
[task 2020-03-30T11:14:40.349Z] 11:14:40     INFO - Console message: [JavaScript Error: "getScreenshot(http://mochi.test:8888/browser/devtools/client/netmonitor/test/html_ws-test-page.html) failed: TypeError: NetworkError when attempting to fetch resource." {file: "resource://activity-stream/lib/Screenshots.jsm" line: 59}]
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - getScreenshotForURL@resource://activity-stream/lib/Screenshots.jsm:59:10
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - async*maybeCacheScreenshot@resource://activity-stream/lib/Screenshots.jsm:112:37
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - _fetchScreenshot@resource://activity-stream/lib/TopSitesFeed.jsm:527:23
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - _fetchIcon@resource://activity-stream/lib/TopSitesFeed.jsm:515:16
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - getLinksWithDefaults@resource://activity-stream/lib/TopSitesFeed.jsm:408:16
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - async*refresh@resource://activity-stream/lib/TopSitesFeed.jsm:431:30
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - onAction@resource://activity-stream/lib/TopSitesFeed.jsm:771:14
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - _middleware/</<@resource://activity-stream/lib/Store.jsm:63:17
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - Store/this[method]@resource://activity-stream/lib/Store.jsm:39:54
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - init/this.intervalId<@resource://activity-stream/lib/SystemTickFeed.jsm:27:24
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - notify@resource://gre/modules/Timer.jsm:62:17
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - 
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - Buffered messages finished
[task 2020-03-30T11:14:40.350Z] 11:14:40     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_ws-early-connection.js | Test timed out - 
[task 2020-03-30T11:14:41.205Z] 11:14:41     INFO - Removing tab.
[task 2020-03-30T11:14:41.205Z] 11:14:41     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2020-03-30T11:14:41.220Z] 11:14:41     INFO - Got event: 'TabClose' on [object XULElement].
[task 2020-03-30T11:14:41.235Z] 11:14:41     INFO - GECKO(2400) | [Parent 5336, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorService.cpp, line 182
[task 2020-03-30T11:14:41.235Z] 11:14:41     INFO - GECKO(2400) | [Parent 5336, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorService.cpp, line 182
[task 2020-03-30T11:14:41.242Z] 11:14:41     INFO - Tab removed and finished closing
[task 2020-03-30T11:14:41.277Z] 11:14:41     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_ws-early-connection.js | The main process DevToolsServer has no pending connection when the test ends - 
[task 2020-03-30T11:14:41.277Z] 11:14:41     INFO - finish() was called, cleaning up...
[task 2020-03-30T11:14:41.280Z] 11:14:41     INFO - GECKO(2400) | [Parent 5336: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 1ED59000 == 6 [pid = 5336] [id = {4744dd9f-d256-4119-888c-42c9f2166834}] [url = chrome://devtools/content/netmonitor/index.html]
[task 2020-03-30T11:14:41.315Z] 11:14:41     INFO - GECKO(2400) | MEMORY STAT | vsize 1231MB | vsizeMaxContiguous 205MB | residentFast 490MB | heapAllocated 144MB
[task 2020-03-30T11:14:41.315Z] 11:14:41     INFO - TEST-OK | devtools/client/netmonitor/test/browser_net_ws-early-connection.js | took 91111ms
Flags: needinfo?(kishlaya.j)

Kishlaya, one test needs to be updated. See this place:
https://searchfox.org/mozilla-central/rev/fa2df28a49883612bd7af4dacd80cdfedcccd2f6/devtools/client/netmonitor/test/browser_net_ws-early-connection.js#56

The test waits for four frames (despite the comment) including Control frames. Since Control frames are not visible by default it should wait for two frames (just like the comment says).

Honza

Flags: needinfo?(victoria)

Thanks Honza, I have made the changes you mentioned.

But I have a couple of question here:

  1. I thought the error is because of line #38 which says there should be 3 requests but 4 were received.

  2. Because of the above issue, I wanted to run the tests locally and see what are those 4 requests, but I couldn't find any documentation on testing and in particular, how are tests being executed.

Flags: needinfo?(kishlaya.j) → needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5431843c523d
Added another filter frame type for viewing control frames separately r=Honza
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 76
Flags: needinfo?(odvarko)

The Messages tab was not documented, so I added a Messages section to the "Network request details" article. Sorry, I don't have a good example to use for screen shots, so I didn't add any. Honza, please verify that my descriptions are correct.

Flags: needinfo?(odvarko)

(In reply to Janet Swisher from comment #29)

The Messages tab was not documented, so I added a Messages section to the "Network request details" article. Sorry, I don't have a good example to use for screen shots, so I didn't add any. Honza, please verify that my descriptions are correct.

The section looks great to me. I think we could yet link to the Inspecting web sockets page that has more detailed info.

I also created a test case page for WS inspection here:
http://janodvarko.cz/test/websockets/
You can use it to create screenshots if needed.

Thanks Janet!

Honza

Flags: needinfo?(odvarko)

I had not realized there was a page one Inspecting web sockets. I move the description of the message filter menu into the Filtering web socket frames section of that page, and added a screenshot. I replaced that info on the Monitor request details page with a link to the "Inspecting" page.

Flags: needinfo?(jswisher)
You need to log in before you can comment on or make changes to this bug.