765651 - WebSocket connections are listed with the "http:" protocol in the netmonitor/console

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Comment 1

[Thaddee Tyl [:espadrine]]

Obviously, Websocket connections should be listed as "ws://…" (or "wss://" for secured connections).
However, they are listed as "http://localhost/$socket.io/1/websocket/15713794881205045864", for instance.

Comment 2

[Brandon Benvie [:benvie]]

Check if the protocol is still actually shown (it my not be).

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Honza, it still happens in netmonitor and console. Do you think it could be a good first bug?

Comment 4

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

Andrea, the request displaying in the list is the original HTTP upgrade request (coming from nsIHttpChannel.URL.spec).
How can we get URL of the actual WS connection?

Honza

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

STR:

1. Go to http://janodvarko.cz/test/websockets/
2. Open netmonitor
3. Click on the Connect button

Comment 6

[Andrea Marchesini [:baku]]

Valentin, do you know how to obtain a nsIWebSocketChannel from a nsIHttpChannel?

Comment 7

[Valentin Gosu [:valentin] (he/him)]

(In reply to Andrea Marchesini [:baku] from comment #6)

Valentin, do you know how to obtain a nsIWebSocketChannel from a nsIHttpChannel?

I presume this is in the parent process.
The WSChannel sets itself as the notification callbacks so I suppose a simple NS_QueryNotificationCallbacks should suffice.

Comment 8

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

Valentin: I tried the following:

diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -660,16 +660,23 @@ NetworkObserver.prototype = {
     if (channel.loadInfo) {
       causeType = channel.loadInfo.externalContentPolicyType;
       const { loadingPrincipal } = channel.loadInfo;
       if (loadingPrincipal && loadingPrincipal.URI) {
         causeUri = loadingPrincipal.URI.spec;
       }
     }

+    if (channel.notificationCallbacks) {
+      let wsChannel = channel.notificationCallbacks.QueryInterface(Ci.nsIWebSocketChannel);
+      if (wsChannel) {
+        event.url = wsChannel.URI.spec;
+      }
+    }
+
     event.cause = {
       type: causeTypeToString(causeType),
       loadingDocumentUri: causeUri,
       stacktrace,
     };

     httpActivity.isXHR = event.isXHR =
       causeType === Ci.nsIContentPolicy.TYPE_XMLHTTPREQUEST ||

But, I am seeing an exception when connecting to WS:
TypeError: "channel.notificationCallbacks.QueryInterface is not a function"

What am I doing wrong?

Honza

Comment 9

[Valentin Gosu [:valentin] (he/him)]

notificationCallbacks is a nsIInterfaceRequestor. You should call getInterface on it, not QueryInterface.

Comment 10

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

I tried the following:

diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -666,16 +666,23 @@ NetworkObserver.prototype = {
     if (channel.loadInfo) {
       causeType = channel.loadInfo.externalContentPolicyType;
       const { loadingPrincipal } = channel.loadInfo;
       if (loadingPrincipal && loadingPrincipal.URI) {
         causeUri = loadingPrincipal.URI.spec;
       }
     }

+    if (channel.notificationCallbacks) {
+      let wsChannel = channel.notificationCallbacks.getInterface(Ci.nsIWebSocketChannel);
+      if (wsChannel) {
+        event.url = wsChannel.URI.spec;
+      }
+    }
+
     event.cause = {
       type: causeTypeToString(causeType),
       loadingDocumentUri: causeUri,
       stacktrace,
     };

     httpActivity.isXHR = event.isXHR =
       causeType === Ci.nsIContentPolicy.TYPE_XMLHTTPREQUEST ||

But, I am still seeing an error:

Exception { name: "NS_NOINTERFACE", message: "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIInterfaceRequestor.getInterface]", result: 2147500034, filename: "resource://devtools/server/actors/network-monitor/network-observer.js", lineNumber: 678, columnNumber: 0, data: null, stack: "_createNetworkEvent@resource://devtools/server/actors/network-monitor/network-observer.js:678:53\n_onRequestHeader@resource://devtools/server/actors/network-monitor/network-observer.js:764:10\nNetworkObserver.prototype.observeActivity<@resource://devtools/server/actors/network-monitor/network-observer.js:577:12\nexports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:111:22\n", location: XPCWrappedNative_NoHelper }
ThreadSafeDevToolsUtils.js:90:13

Comment 11

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #10)
Hi Honza,

I got some code that seems to work on the page in comment 5.

diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network-observer.js
--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -664,6 +664,15 @@ NetworkObserver.prototype = {
         causeUri = loadingPrincipal.URI.spec;
       }
     }
+    if (channel.notificationCallbacks) {
+      let wsChannel = null;
+      try {
+        wsChannel = channel.notificationCallbacks.QueryInterface(Ci.nsIWebSocketChannel);
+      } catch(e) {}
+      if (wsChannel) {
+        event.url = wsChannel.URI.spec;
+      }
+    }
 
     event.cause = {
       type: causeTypeToString(causeType),

I think we need the try catch bit because not all channels can be QId to nsIWebSocketChannel.

Comment 12

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

Thanks Valentin, this works for me!
Honza

Comment 13

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=566f28ffdb360308d2a76f53510e310b58f90c28

Comment 14

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

Instructions for anyone interested in fixing this bug:

1. This is the piece that get the original WS url:

diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network-observer.js
--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -666,16 +666,31 @@ NetworkObserver.prototype = {
     if (channel.loadInfo) {
       causeType = channel.loadInfo.externalContentPolicyType;
       const { loadingPrincipal } = channel.loadInfo;
       if (loadingPrincipal && loadingPrincipal.URI) {
         causeUri = loadingPrincipal.URI.spec;
       }
     }
 
+    // Show the right WebSocket URL in case of WS channel.
+    if (channel.notificationCallbacks) {
+      let wsChannel = null;
+      try {
+        wsChannel = channel.notificationCallbacks.QueryInterface(
+          Ci.nsIWebSocketChannel
+        );
+      } catch (e) {
+        // Not all channels implement nsIWebSocketChannel.
+      }
+      if (wsChannel) {
+        event.url = wsChannel.URI.spec;
+      }
+    }
+
     event.cause = {
       type: causeTypeToString(causeType),
       loadingDocumentUri: causeUri,
       stacktrace,
     };
 
     httpActivity.isXHR = event.isXHR =
       causeType === Ci.nsIContentPolicy.TYPE_XMLHTTPREQUEST ||

2. We also need to fix couple of test:

• devtools/client/netmonitor/test/browser_net_filter-01.js
• devtools/client/netmonitor/test/browser_net_websocket_stacks.js

It should be about expecting ws and not http in tests.

See here:

• https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/devtools/client/netmonitor/test/browser_net_websocket_stacks.js#23
• https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/devtools/client/netmonitor/test/browser_net_filter-01.js#198

Note that WS_CONTENT_TYPE_SJS uses http: so, we need another constant based on WS_URL

https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/devtools/client/netmonitor/test/head.js#47

Honza

Comment 15

[Thiago Arrais]

Can I tackle this one?

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Sure Thiago!

You may want to read this in order to set up the work environment: http://docs.firefox-dev.tools/getting-started/

I'm going to assign the bug to you so other people know it's not available anymore.

Feel free to ask any question, either here, or on Slack :)

Comment 17

[Thiago Arrais]

Attachment: Bug 765651 - Fix console protocol for ws connections. r=Honza,nchevobbe

Comment 18

[Thiago Arrais]

Just opened a review. This is my first contribution to FF, though, and I'm not sure if I did everything right. Please let me know if you need anything else.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa40b800b2ce
Fix console protocol for ws connections. r=Honza,nchevobbe

Comment 20

[Thiago Arrais]

Not sure what is needed here. I'm running devtools/client/netmonitor/test/browser_net_websocket_stacks.js locally and can't reproduce the regression.

Can anyone point me in the right direction?

Comment 21

[Natalia Csoregi [:nataliaCs]]

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