1587750 - The JSON section is empty even if there are content

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[Raluca Popovici, Desktop QA]

Attachment: 2019-10-10_09h48_36.png

Preconditions: Set 'devtools.netmonitor.features.webSockets' to "true" in about:config
Restart the browser. (Nightly 71)

Steps to reproduce:

1. Launch Firefox browser
2. Navigate to: https://socket.io/#examples
3. Open Devtools(press F12)
4. Go to the Network tab
5. Refresh the page from Step 2
6. Click on "https://socket-io-tweet-stream.now.sh/socket.io/?EIO=3&transport=websocket&sid=t0ECsWx1vROhV2PKAYlP" request
7. Click on "Messages" panel from the right side
8. Select a frame which contains JSON data

Actual result:
The details data is opened. The JSON section is empty even if there is 1B.

Expected result:
The JSON section shouldn't be empty

Comment 1

[Honza Bambas (:mayhemer)]

This is I believe a DevTools issue.

Comment 2

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

Thanks for the report, I can also see the issue on my machine (Win10)
Honza

Comment 3

[:Harald Kirschner :digitarald]

This seems to happen for numbers, which probably pass whatever "JSON test" we are running.

Comment 4

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

Some pointers to the code base for anyone interested in fixing this bug:

1. The WS payload is parser here:
   https://searchfox.org/mozilla-central/rev/17756e2a5c180d980a4b08d99f8cc0c97290ae8d/devtools/client/netmonitor/src/components/websockets/FramePayload.js#111

As Harald points in comment #3 primitives like a number or string in quotes or boolean values etc. pass the parsing.

2. The isJSON helper comes from here:
   https://searchfox.org/mozilla-central/rev/17756e2a5c180d980a4b08d99f8cc0c97290ae8d/devtools/client/netmonitor/src/utils/request-utils.js#607

I think that we should change the isJSON helper by adding something like as follows in it:

if (parseFloat(payload) || (payload.startsWith("\"") && payload.endsWith("\""))) {
  return {json, error};
}

This way primitives won't be treated as JSON.
Modifying the helper will also fix weird behavior on the Response panel where JSON section is also created for those and the value is displayed directly in the header, which is not consistent.

Honza

Comment 5

[Mustafa]

can I work on it?

Comment 6

[Mustafa]

Attachment: Bug 1587750 - fix isJSON healper function. r=Honza

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af6c810846b4
fix isJSON healper function. r=Honza

Comment 8

[Dorel Luca [:dluca]]

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

Comment 9

[:Harald Kirschner :digitarald]

Comment on attachment 9101284 [details]
Bug 1587750 - fix isJSON healper function. r=Honza

Beta/Release Uplift Approval Request

• User impact if declined: Slight breakage in a new keystone feature (WebSocket Inspector) during initial release.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple patch for that only targets the preview part for WebSockets.
• String changes made/needed:

Comment 10

[Pascal Chevrel:pascalc]

Comment on attachment 9101284 [details]
Bug 1587750 - fix isJSON healper function. r=Honza

Minimal low-risk patch, fix for a new devtools feature, uplift approved for 71 beta 6, thanks.

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/482ca58d5382