Closed Bug 765651 Opened 8 years ago Closed 2 months ago

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

Categories

(DevTools :: Console, defect, P3)

15 Branch
defect

Tracking

(firefox73 fixed)

RESOLVED FIXED
Firefox 73
Tracking Status
firefox73 --- fixed

People

(Reporter: espadrine, Assigned: thiago.arrais)

References

(Regressed 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

No description provided.
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.
Priority: -- → P3
Check if the protocol is still actually shown (it my not be).
Depends on: WSConsoleLogging
Product: Firefox → DevTools

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

Flags: needinfo?(odvarko)

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

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

STR:

  1. Go to http://janodvarko.cz/test/websockets/
  2. Open netmonitor
  3. Click on the Connect button
Summary: WebSocket connections are listed with the "http:" protocol in the console. → WebSocket connections are listed with the "http:" protocol in the console (and netmonitor)

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

Flags: needinfo?(amarchesini) → needinfo?(valentin.gosu)

(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.

Flags: needinfo?(valentin.gosu)

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

Flags: needinfo?(valentin.gosu)

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

Flags: needinfo?(valentin.gosu)

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

Flags: needinfo?(valentin.gosu)

(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.

Flags: needinfo?(valentin.gosu)

Thanks Valentin, this works for me!
Honza

Summary: WebSocket connections are listed with the "http:" protocol in the console (and netmonitor) → WebSocket connections are listed with the "http:" protocol in the netmonitor/console

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 ||
  1. 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:

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

Keywords: good-first-bug

Can I tackle this one?

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 :)

Assignee: nobody → thiago.arrais
Status: NEW → ASSIGNED

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.

Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aa40b800b2ce
Fix console protocol for ws connections. r=Honza,nchevobbe
Regressions: 1604202

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?

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 73
You need to log in before you can comment on or make changes to this bug.