1566521 - Indicate that WS connection is closed

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

There should be a message displayed at the end of the message list (if WS frame list) indicating that the (WS) connection has been closed.

https://www.figma.com/file/OWflRU1bKHXMR6nh921AQiMD/Network-WebSocket-Inspector?node-id=1%3A5

Honza

Comment 2

[:Harald Kirschner :digitarald]

Closing a WS can provide a code and/or reason that could be displayed here as well: https://html.spec.whatwg.org/multipage/web-sockets.html#the-websocket-interface:dom-websocket-close

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yep, we have this callback:
https://searchfox.org/mozilla-central/rev/4fbcfe6fecf75d7b828fbebda79a31385f7eab5f/devtools/client/netmonitor/src/connector/firefox-data-provider.js#457

  /**
   * The "webSocketClosed" message type handler.
   *
   * @param {boolean} wasClean
   * @param {number} code
   * @param {string} reason
   */
  async onWebSocketClosed(wasClean, code, reason) {}

code & reason are described in the spec, but there is also wasClean.

Baku, what the wasClean argument indicates?
Something interesting for our users?

Honza

Comment 4

[Andrea Marchesini [:baku]]

'wasClean' is set to false when the webSocket is terminated because of an error. For instance, the page is reloaded, or a network failure can trigger a 'wasClean'=false close event.

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

Attched screenshot showing the notification about truncated messages.

@Harald, do we want something (visually) similar for the "connection closed" notification?

Honza

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

Ah, just found the Figma link.

Mockup showing how to message should look like attached.

Honza

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Some instructions for anyone interested in fixing this bug

1. The message should be rendered at the end of the WS frame list.
   The list is rendered here:
   https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/client/netmonitor/src/components/websockets/FrameListContent.js#320

2. Here is the backend place where we have a hook for observing closed notifications
   https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/server/actors/network-monitor/websocket-actor.js#92

The method emits an event that is sent to the client side (front end, web socket panel UI):
this.emit("serverWebSocketClosed", wasClean, code, reason);

It needs to also send "httpChannelId" so, the client side can identify, which websocket connection is closed.
It can be done as follows:
const httpChannelId = this.connections.get(webSocketSerialID);
(see frameReceived method in the same file)

Also, RDP packet spec needs to be updated if new field is added
https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/shared/specs/websocket.js#23

3. Here is a callback executed on the client side (when receiving the emitted packet)
   https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/client/netmonitor/src/connector/firefox-data-provider.js#460

It should have new first argument httpChannelId
async onWebSocketClosed(httpChannelId, wasClean, code, reason) {}

The method should fire a new Redux action, call something like:

if (this.actionsEnabled && this.actions.closeConnection {
  await this.actions.closeConnection(httpChannelId, wasClean, code, reason);
}

The action should be implemented (and exported) in this file
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/actions/web-sockets.js

4. The action should be handled in WebSocket reducer
   https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/client/netmonitor/src/reducers/web-sockets.js#189

We could introduce a new map: closedConnections httpChannelId => boolean, where true == closed
in the reducer

Consequently we could implement a selector isCurrentChannelClosed in this file:
https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/client/netmonitor/src/selectors/web-sockets.js

5. Finally, we could access the selector in the connect method here
   https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/client/netmonitor/src/components/websockets/FrameListContent.js#342

and call is as follows:

  state => ({
    selectedFrame: getSelectedFrame(state),
    frames: getDisplayedFrames(state),
    columns: state.webSockets.columns,
    isClosed: isCurrentChannelClosed(state),
  }),

The isClosed property would be utilized in the render method (see step #1) and if true, the final "Connection closed" message would be rendered (perhaps together with a reason). The message (not the reason) should be properly localized.

Honza

Comment 8

[Sai Hemanth]

Hey Honza,
I'd like to work on this as my first bug. I've worked with this stack before and the fix is pretty clear. Can I get the repo link for the same? I've looked at mozilla-central but it seems like that's not being used.

saihemanth9019

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to saihemanth9019 from comment #8)

I'd like to work on this as my first bug. I've worked with this stack before and the fix is pretty clear.

Excellent, thanks!

Can I get the repo link for the same? I've looked at mozilla-central but it seems like that's not being used.

Not sure if I understand the question, you need to clone mozilla-central: hg clone https://hg.mozilla.org/mozilla-central
You can read these instructions: https://docs.firefox-dev.tools/getting-started/build.html

Honza

Comment 10

[Sai Hemanth]

Understood! Working on it.

Comment 11

[Sai Hemanth]

Attachment: Bug 1566521 - Indicate that WS connection is closed. r=honza

Comment 12

[Sai Hemanth]

I've made the changes and submitted a revision. Added 'honza' as the reviewer.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c84f90739d8e
Indicate that WS connection is closed. r=Honza,Harald

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c84f90739d8e