1590046 - Formatting for Action Cable WebSocket messags

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[:Harald Kirschner :digitarald]

Reference: https://edgeguides.rubyonrails.org/action_cable_overview.html

I have so far not found test pages or specific on the format.

Comment 1

[:Harald Kirschner :digitarald]

Looks like action cables can contain serialized JSON inside JSON values: https://medium.com/codequest/actioncable-in-rails-api-f087b65c860d

{
    "command": "message",
    "identifier": "{\"channel\": \"MyChannel\"}",
    "data": 
        "{\"action\": \"some_method\", \"args\": \"{\\\"arg_1\\\": 1}\"}"
}

Looking at the code.

Only for outgoing data, send does JSON encode data and identifier which both in one object are later encoded again to JSON: https://github.com/rails/rails/blob/d03177ffbccb5b30217448d526f1e9aa1fa24297/actioncable/app/javascript/action_cable/subscription.js#L83 .

The consumer code does not parse anything extra, so maybe incoming messages do not include JSON in strings but I am not sure as one test compares fields based on JSON.stringify: https://github.com/rails/rails/blob/85b080365313437f646070ca214fef433c06db6a/actioncable/test/javascript/src/unit/subscription_test.js#L48

So far I assume we just need to take special care of those two properties, data and identifier.
The code for parsing responses looks straightforward with few options to handle: https://github.com/rails/rails/blob/master/actioncable/app/javascript/action_cable/connection.js#L122-L141

Comment 2

[Cam]

Here's some samples from an action cable app I use:

↓ {"type":"welcome"}
↑ {"command":"subscribe","identifier":"{\"channel\":\"RoomChannel\",\"room\":\"main\"}"}
↓ {"identifier":"{\"channel\":\"RoomChannel\",\"room\":\"main\"}","type":"confirm_subscription"}
↓ {"identifier":"{\"channel\":\"RoomChannel\",\"room\":\"main\"}","message":{"action":"joined","user":"[e-mail]"}}
↓ {"type":"ping","message":1571706406}
↑ {"command":"message","identifier":"{\"channel\":\"RoomChannel\",\"room\":\"main\"}","data":"{\"room\":\"main\",\"message\":\"hi\"}"}
↓ {"identifier":"{\"channel\":\"RoomChannel\",\"room\":\"main\"}","message":{"action":"message","user":"[e-mail]","message":"hi"}}

Comment 3

[:Harald Kirschner :digitarald]

Good first bug. The format seems simple enough to detect, and only 2 fields that need JSON treatment.

Instructions on adding now formatters are provided by Honza here: https://bugzilla.mozilla.org/show_bug.cgi?id=1589068#c2

Comment 4

[Uday Mewada]

hi Harald, I would like to work on it.

Comment 5

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

Assigned to you.
Thanks for the help!

Honza

Comment 6

[Uday Mewada]

As far as my understanding goes, here is what I understood(forgive me if I am wrong anywhere),
as referred by Honza here
I found that ParsePayload contains socket.io, sockjs & json payload parser
and from my guess, I need to implement SignalR parser in the ParsePayload method.

Comment 7

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

(In reply to Uday Mewada from comment #6)

and from my guess, I need to implement SignalR parser in the ParsePayload method.

Support for SignalR is already implemented in bug 1589068 and you can see it in the source here:
https://searchfox.org/mozilla-central/rev/b1e51ad5613ad3d911cf42e3e525885ce2278915/devtools/client/netmonitor/src/components/websockets/FramePayload.js#154

This bug is about support for Action Cable
https://edgeguides.rubyonrails.org/action_cable_overview.html

Honza

Comment 8

[:Harald Kirschner :digitarald]

In comment 1 and comment 3 I pointed out that additional parsing needs to be done on 2 fields in the response. I don't think an MVP of this needs to pull in additional libraries. https://thoughtbot.com/blog/talking-to-actioncable-without-rails has an overview of the basics that explains more of the format as well.

Comment 9

[Uday Mewada]

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

In comment 1 and comment 3 I pointed out that additional parsing needs to be done on 2 fields in the response. I don't think an MVP of this needs to pull in additional libraries. https://thoughtbot.com/blog/talking-to-actioncable-without-rails has an overview of the basics that explains more of the format as well.

Sir! In WebSocket inspector in the network panel, we can see messages but we can't see anything in response panel.
'I gathered this info:' when data goes through action cable it is encoded and what I have to do is decode that message and present it to the user.
But I am not getting anything how should I proceed.

Comment 10

[:Harald Kirschner :digitarald]

Sir! In WebSocket inspector in the network panel, we can see messages but we can't see anything in response panel.

You have to select the Messages tab. Please make yourself comfortable with how the WebSocket inspector works following https://hacks.mozilla.org/2019/10/firefoxs-new-websocket-inspector/ and examples pages like https://firefox-devtools-debug-ws.glitch.me/

Comment 11

[Uday Mewada]

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

Sir! In WebSocket inspector in the network panel, we can see messages but we can't see anything in response panel.

You have to select the Messages tab. Please make yourself comfortable with how the WebSocket inspector works following https://hacks.mozilla.org/2019/10/firefoxs-new-websocket-inspector/ and examples pages like https://firefox-devtools-debug-ws.glitch.me/

As you referred me here: https://hacks.mozilla.org/2019/10/firefoxs-new-websocket-inspector/, I found that How to see different information about any HTTP request which we are making and inspect them through WebSocket inspector and different protocol which we are using in the inspector.
In the Messages tab, when we click on any message, it is visible in two formats JSON & Raw Data.

Comment 12

[:Harald Kirschner :digitarald]

I recommend to check out how SignalR support got added: https://phabricator.services.mozilla.com/D51498

I posted a few links in other comments on what's needed to improve parsing for ActionCable (basically parsing values for 2 fields in the JSON), which can follow SignalR's example on detecting + parsing messages.

Let me know if you have any specific questions.

Comment 13

[Uday Mewada]

on comment #12 you referred me on this: https://phabricator.services.mozilla.com/D51498, Here I am somehow able to understand how the things are happening.
But still, I am not getting what I have to do?
Do I change the format in which the messages are shown in the message tab?

Comment 14

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

Hey Uday, let me explain more what's expected to be done in this report.

Firefox DevTools allows the user to inspect WebSocket (WS) traffic. In other words, the user is able to see data (payload) of all WS frames sent or recieved. This can be done in the Messages side panel.

Data sent through WS frames are often using custom format. It can be plain text, JSON, or more sophisticated protocols for Socket.IO, SockJS or SignalR. We want to make the inspection feature even better for these specific protocols, by parsing the data (sent through WS frames) and displaying them as expandable tree.

We already supports those protocols (i.e. we already have parsers for the above protocols) and this bug is about
supporting a new one for: Action Cable

So, to support a new protocol, we need to introduce a new parser. In this bug we need to introduce a parser for Action Cable WS frame payload.

There is great information about the protocol (for Action Cable) in comment #1 and there are some existing instructions showing how to integrate new parsers with the existing infrastructure here: https://bugzilla.mozilla.org/show_bug.cgi?id=1589068#c2 <- this link points to a bug that was about supporting SignalR, it's already fixed. But, the instructions are useful in this bug too.

I was looking at this bug scope again and it might be more complex and doesn't feel as ideal good first bug. You might want to look around for other first-good-bugs and get some more experience first. I see that you are already assigned to another two bugs so, what about fixing them first?

Honza

Comment 15

[Uday Mewada]

sir, I want to give it one more try.
As you told I have to introduce ActionCable parser in parsePayload. I have seen in the parsers directory have different parsers and each parser contains different files. So, which files should I include in Action Cable parser?
Are the files similar to which are included in SignalR? Only the difference in the properties to be included like https://github.com/rails/rails/blob/master/actioncable/app/javascript/action_cable/connection.js#L123-L138 in the Action Cable parser
Or I have to include different files.

Comment 16

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

(In reply to Uday Mewada from comment #15)

sir, I want to give it one more try.
As you told I have to introduce ActionCable parser in parsePayload.

Correct

I have seen in the parsers directory have different parsers and each parser contains different files.

Correct

So, which files should I include in Action Cable parser?
Are the files similar to which are included in SignalR?

Different parsers -> different files

Only the difference in the properties to be included like https://github.com/rails/rails/blob/master/actioncable/app/javascript/action_cable/connection.js#L123-L138 in the Action Cable parser
Or I have to include different files.

These are two different parsers.
SignalR is a library for ASP.NET developers and Action Cable is for Rails developers.

You can look at the comment #1 (there are couple of good links) and understand how the Action Cable protocol works and how to include proper files for parsing it into Firefox DevTools. Doing these analysis is part of the work on this bug.

Honza

Comment 17

[:Harald Kirschner :digitarald]

Found an example page for testing: https://action-cable-example.herokuapp.com/

Comment 18

[:Harald Kirschner :digitarald]

Attachment: actioncable-draft.patch

This is a working patch to get you started. This could be submitted as-is and might just need minor cleanup.

For JSON formatted messages it checks if data and/or identifier are JSON-formatted; parses them and puts the parsed data back into the payload. ActionCable is not a complex format and those 2 fields are the only ones that need extra treatment.

Looking for data fields in JSON is very broad. Maybe the rule can be tightened by adding a some other property exists + data is JSON to defer Action Cable with higher confidence.

Comment 19

[Uday Mewada]

sir, now basically I have to search one property and apply it into if() statement, with the help of that property we can ensure that it is of Action Cable.

Comment 20

[Uday Mewada]

sir, What if do add some other properties like message_type like as mentioned here: https://github.com/rails/rails/blob/master/actioncable/app/javascript/action_cable/connection.js#L122-L141 in if() statement. Like,
if(actionCablePayload && actionCablePayload.message_type=="ping")
if(actionCablePayload && actionCablePayload.message_type=="welcome"), because ones the execution into if(json) then we can the use property message_type.

Comment 21

[:Harald Kirschner :digitarald]

The key would be type. I am not sure how reliable type would be given the example from comment 2, identifier also comes with command, type or message. Maybe checking if any of these keys is present as well might be a good-enough safeguard.

On the other side, maybe this is good enough as it is. It does just decode JSON embedded in JSON. If anybody really encodes JSON objects in JSON WebSocket message and names those fields identifier and data, decoding them isn't bad and calling it Action Cable is only slightly misleading. Do you want to go ahead and make a patch for review, Uday?

Comment 22

[Uday Mewada]

Attachment: Bug 1590046 - Formatting for Action Cable WebSocket messages. r=Harald

Comment 23

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

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

On the other side, maybe this is good enough as it is. It does just decode JSON embedded in JSON. If anybody really encodes JSON objects in JSON WebSocket message and names those fields identifier and data, decoding them isn't bad and calling it Action Cable is only slightly misleading.

I agree, this should work in most cases.

Honza

Comment 24

[Uday Mewada]

Attachment: Bug 1590046 - Formatting for Action Cable WebSocket messages. r=Harald

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/715bb381d7eb
Formatting for Action Cable WebSocket messages. r=Harald,Honza

Comment 26

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/715bb381d7eb