Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node --inspect sending ints as floats #7736

Closed
nojvek opened this issue Jul 14, 2016 · 2 comments · Fixed by #7796
Closed

node --inspect sending ints as floats #7736

nojvek opened this issue Jul 14, 2016 · 2 comments · Fixed by #7796
Labels
inspector Issues and PRs related to the V8 inspector protocol

Comments

@nojvek
Copy link

nojvek commented Jul 14, 2016

  • Version: v7.0.0-nightly20160712ef1f7661c7
  • Platform: mac
  • Subsystem:

Here's what a typical start of a websocket session looks like

creating ws://localhost:9229/node
> {"id":1,"method":"Runtime.enable"}
> {"id":2,"method":"Debugger.enable"}
> {"id":3,"method":"Console.enable"}
> {"id":4,"method":"Profiler.enable"}
> {"id":5,"method":"HeapProfiler.enable"}
> {"id":6,"method":"Runtime.run"}
< {"method":"Runtime.executionContextCreated","params":{"context":{"id":1.000000,"isDefault":true,"origin":"","name":"NodeJS Main Context","frameId":""}}}
< {"id":1.000000,"result":{}}
< {"id":2.000000,"result":{}}
< {"error":{"code":-32601.000000,"message":"'Console.enable' wasn't found"},"id":3.000000}
< {"id":4.000000,"result":{}}
< {"id":5.000000,"result":{}}
< {"id":6.000000,"result":{}}
< {"method":"Debugger.scriptParsed","params":{"scriptId":"39","url":"bootstrap_node.js","startLine":0.000000,"startColumn":0.000000,"endLine":457.000000,"endColumn":0.000000,"executionContextId":1.000000,"hash":"D0A4F4210878862AC73FFB6658F7EC021A33D3F4","isContentScript":false,"isInternalScript":false,"isLiveEdit":false,"sourceMapURL":"","hasSourceURL":false,"deprecatedCommentWasUsed":false}}

Expected

  • Console.enable should not error since console is in the js_protocol.json domain
  • Response id's should be whole numbers not 1.0000. It doesn't break javascript but adds extra bytes on the websocket pipe when can be avoided. The other ids like startLine, endLine and executionContextId also have similar issues which can add up.

Chrome has the same problem so I'm guessing it needs to be fixed in v8 land.

/cc @pavelfeldman @ofrobots

@mscdex mscdex added the inspector Issues and PRs related to the V8 inspector protocol label Jul 14, 2016
@pavelfeldman
Copy link
Contributor

@nojvek
Copy link
Author

nojvek commented Jul 14, 2016

Filed the console issue separate as well

https://bugs.chromium.org/p/chromium/issues/detail?id=628390

ofrobots added a commit to ofrobots/node that referenced this issue Jul 21, 2016
To add a LICENSE file along with the v8_inspector code, we need to
pick up v8_inspector from an intermediate repository:
https://github.com/pavelfeldman/v8_inspector. This repo still tracks
upstream code in Blink.

This roll also picks up the latest v8_inspector from upstream fixing
a few issues.

* Pickup commit id bc60957 from pavelfeldman/v8_inspector
* Update node.gyp to adapt to the new file paths
* Update the DevTools hash for the devtools frontend.

Fixes: nodejs#7123
Fixes: nodejs#7736
Fixes: nodejs#7734
ofrobots added a commit to ofrobots/node that referenced this issue Jul 27, 2016
To add a LICENSE file along with the v8_inspector code, we need to
pick up v8_inspector from an intermediate repository:
https://github.com/pavelfeldman/v8_inspector. This repo still tracks
upstream code in Blink.

This roll also picks up the latest v8_inspector from upstream fixing
a few issues.

* Pickup commit id bc60957 from pavelfeldman/v8_inspector
* Update node.gyp to adapt to the new file paths
* Update the DevTools hash for the devtools frontend.

Fixes: nodejs#7123
Fixes: nodejs#7736
Fixes: nodejs#7734
PR-URL: nodejs#7796
Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Reviewed-By: jasnell - James M Snell <[email protected]>
cjihrig pushed a commit that referenced this issue Aug 11, 2016
To add a LICENSE file along with the v8_inspector code, we need to
pick up v8_inspector from an intermediate repository:
https://github.com/pavelfeldman/v8_inspector. This repo still tracks
upstream code in Blink.

This roll also picks up the latest v8_inspector from upstream fixing
a few issues.

* Pickup commit id bc60957 from pavelfeldman/v8_inspector
* Update node.gyp to adapt to the new file paths
* Update the DevTools hash for the devtools frontend.

Fixes: #7123
Fixes: #7736
Fixes: #7734
PR-URL: #7796
Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Reviewed-By: jasnell - James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants