Closed Bug 1619026 Opened 3 years ago Closed 3 years ago

WAMP fails with JSON error for binary messages

Categories

(DevTools :: Netmonitor, defect, P4)

Product:
DevTools
Component:
Netmonitor
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 1621822

People

(Reporter: Harald, Unassigned)

References

Details

What were you doing?

  1. Open Network panel on https://crossbar.io/ and select the WS
  2. Inspect binary messages, like 1dTm

What happened?

UI doesn't update.

16:23:54.078 SyntaxError: JSON.parse: unexpected keyword at line 1 column 1 of the JSON data serializers.js:36:17

Which leads to return JSON.parse(payload);

What should have happened?

Preview data, which is probably msgpck.

Anything else we should know?

The client requests wamp.2.json, wamp.2.msgpack while the server responds with wamp.2.json. This might also be a server issue, but lets verify.

Tobias, the server seems to send binary message on a connection that agreed on JSON. Is this something that the parser should handle?

Flags: needinfo?(tobias.oberstein)
Status: NEW → UNCONFIRMED
Ever confirmed: false

These frames don't appear in Chrome. Also other WAMP clients don't seem to receive them. Maybe these are PING/PONG frames or something like that?

Elad

Right, I also see that they didn’t trigger the site onmessage handler (tested using the new event breakpoints), so something is definitely fishy.

Flags: needinfo?(tobias.oberstein)

I can reproduce the issue on my machine.

I am also seeing the following error sin the Browser Console, not sure if it's related:

Error sending event: serverWebSocketClosed Actor.js:68:15
    _sendEvent resource://devtools/shared/protocol/Actor.js:68
    initialize resource://devtools/shared/protocol/Actor.js:46
    _emit resource://devtools/shared/event-emitter.js:226
    emit resource://devtools/shared/event-emitter.js:172
    emit resource://devtools/shared/event-emitter.js:324
    webSocketClosed resource://devtools/server/actors/network-monitor/websocket-actor.js:96


Error: "undefined passed where a value is required"
    identityWrite resource://devtools/shared/protocol/types.js:115
    write resource://devtools/shared/protocol/Request.js:117
    write resource://devtools/shared/protocol/Request.js:43
    _sendEvent resource://devtools/shared/protocol/Actor.js:66
    initialize resource://devtools/shared/protocol/Actor.js:46
    _emit resource://devtools/shared/event-emitter.js:226
    emit resource://devtools/shared/event-emitter.js:172
    emit resource://devtools/shared/event-emitter.js:324
    webSocketClosed resource://devtools/server/actors/network-monitor/websocket-actor.js:96

Honza

Status: UNCONFIRMED → NEW
Ever confirmed: true

In the meantime, I had a chance to test this feature on Firefox 74 release: https://github.com/wamp-proto/wamp-proto/issues/362#issuecomment-597250065

Also, let me comment on a few of the aspects raised in above, as well as add more;)

The client requests wamp.2.json, wamp.2.msgpack while the server responds with wamp.2.json. This might also be a server issue, but lets verify.

This is expected behavior for a WAMP compliant router, as ["wamp.2.json", "wamp.2.msgpack"] is interpreted by the router as a list of WebSocket subprotocol the client is able to talk - ordered by preference! So the router will choose the first one it supports.

These frames don't appear in Chrome. Also other WAMP clients don't seem to receive them. Maybe these are PING/PONG frames or something like that?

Yes, I believe so! The reason is from looking at the message traces here https://github.com/wamp-proto/wamp-proto/issues/362#issuecomment-597487761 - I know this router is doing websocket-server-initiated ping-pong with 10s interval.

If the current code isn't able to handle control frames but tries to treat them as websocket message frames, trying to parse WAMP, be that wamp-json or wamp-msgpack, that must fail. The ping/pong payload that eg Crossbar.io sends is just a configurable chunk of random bytes, that is expected to be replied by the client (Firefox) in a websocket pong. There is nothing to interpret in the ping payload at least for Crossbar.io

However, IMO, it would be cool to just skip control frames without interpretation, because the only case when one might be interested in the websocket control frame level is for maintainers of the websocket implementation within Firefox? Not sure about that .. it would be certainly nice to have control frames show up upon clicking an option in the network inspector .. but, yeah, for me, the 99% case of interest is websocket message(s) / frames .. parsed and shown at the application layer (here, WAMP).

when looking at these lines

https://hg.mozilla.org/mozilla-central/rev/3af87b72a2b4#l1.79

getFramePayload(selectedFrame.payload, connector.getLongString).then(
  async payload => {
      const { formattedData, formattedDataTitle } = await this.parsePayload(
``
* Is this firing for each received websocket message?
* For each control or data message?

If so, then `selectedFrame` can be called with a control frame, and the further processing in `parsePayload` doesn't seem to be ready for control frames?

Is there some `selectedFrame.frame_type` attribute maybe which we could use?

I am guessing a lot in above;) just my 2cts
Depends on: 1621822

Thanks a lot for digging into this, Tobias!

Makes sense that formatting should simply not be applied to control frames. I will track this in a more specific bug. Not showing control frames is tracked in bug 1566780.

Better to dupe it, a this specific issue got a bit convoluted.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE

Makes sense that formatting should simply not be applied to control frames.

yes, I think calling the websocket subprotocol parsing code (like for wamp) only for websocket data traffic, and for whole websocket data messages is much better

I don't know enough about the FF internals, but one thing I would worry about as a consequence of ^:

  • if the inspector should also have the option to show control frames in addition to the subprotocol-parsed data messages, then the inspector traffic pane must be able to show a mix of control frames and parsed data messages, where the former comes from FF common code, while the latter comes from subprotocol-parsing code. Sounds like it could be tricky at least?
  • if the inspector doesn't have the option to show any control traffic at all, everything seems to become straight-forward and simple;)
You need to log in before you can comment on or make changes to this bug.