Closed Bug 1587750 Opened 5 months ago Closed 4 months ago

The JSON section is empty even if there are content

Categories

(DevTools :: Netmonitor, defect, P3)

defect

Tracking

(firefox71 fixed, firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: rpopovici, Assigned: mustafa.abdelmogoud)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(2 files)

Attached image 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

This is I believe a DevTools issue.

Component: Networking: WebSockets → Netmonitor
Product: Core → DevTools

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

Priority: -- → P3

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

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.

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

Keywords: good-first-bug

can I work on it?

Assignee: nobody → mustafa.abdelmogoud
Status: NEW → ASSIGNED
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af6c810846b4
fix isJSON healper function. r=Honza
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

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:
Attachment #9101284 - Flags: approval-mozilla-beta?

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.

Attachment #9101284 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.