inspector: expose original console #21659

mcollina · 2018-07-04T17:57:31Z

Adds require('inspector').console, mapping it to the original
global.console of V8. This enables applications to send messages to
the inspector console programmatically.

Fixes: #21651

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-07-04T17:57:32Z

@mcollina build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/211/pipeline

mcollina · 2018-07-04T17:57:53Z

cc @targos @devsnek

devsnek

you might want to mention that node's console and v8's debug console don't have API parity. (e.g. a method might exist for one that doesn't with the other)

TimothyGu

LGTM after the CI failures have been sorted out.

TimothyGu · 2018-07-04T22:09:35Z

test/sequential/test-inspector-console.js

+async function runTests() {
+ const script = `
+require('inspector').console.log('hello world');
+`;


No need for extraneous new lines.

TimothyGu · 2018-07-04T22:10:05Z

test/sequential/test-inspector-console.js

+ assert.deepStrictEqual(msg.params.args, [{
+ type: 'string',
+ value: 'hello world'
+ }]);


This should also check if stdout is empty.

Trott · 2018-07-05T02:13:01Z

Here's the CI failure:

https://ci.nodejs.org/job/node-test-commit-linuxone/2755/nodes=rhel72-s390x/console

11:08:23 not ok 490 parallel/test-eslint-require-buffer
11:08:23   ---
11:08:23   duration_ms: 0.650
11:08:23   severity: fail
11:08:23   stack: |-
11:08:23     /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:148
11:08:23             throw err;
11:08:23             ^
11:08:23     
11:08:23     AssertionError [ERR_ASSERTION]: 'Use const { Buffer } = require(\'buffer\'); at the beginning of this file' strictEqual 'Use const Buffer = require(\'buffer\').Buffer; at the beginning of this file' ('Use const { Buffer } = require(\'buffer\'); at the beginning of this file' strictEqual 'Use const Buffer = require(\'buffer\').Buffer; at the beginning of this file')
11:08:23         at assertMessageMatches (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:442:24)
11:08:23         at testInvalidTemplate (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:494:29)
11:08:23         at Function.RuleTester.it (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:581:25)
11:08:23         at Function.itDefaultHandler (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:143:23)
11:08:23         at test.invalid.forEach.invalid (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:580:32)
11:08:23         at Array.forEach (<anonymous>)
11:08:23         at Function.RuleTester.describe (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:579:30)
11:08:23         at Function.describeDefaultHandler (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:160:19)
11:08:23         at Function.RuleTester.describe (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:578:24)
11:08:23         at Function.describeDefaultHandler (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:160:19)
11:08:23   ...

Sure seems unrelated. This will need a full CI anyway, so we'll see what happens when a full CI is run. (This was the automatic lite CI.)

mcollina · 2018-07-05T07:23:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15734/

Adds require('inspector').console, mapping it to the original global.console of V8. This enables applications to send messages to the inspector console programmatically. Fixes: nodejs#21651

mcollina · 2018-07-05T08:01:36Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15735/

mcollina · 2018-07-05T09:11:33Z

There are two failures, the first is #21233:

04:46:16 not ok 572 sequential/test-inspector-stop-profile-after-done
04:46:16   ---
04:46:16   duration_ms: 0.739
04:46:16   severity: fail
04:46:16   exitcode: 1
04:46:16   stack: |-
04:46:16     [test] Connecting to a child Node process
04:46:16     [test] Testing /json/list
04:46:16     [err] Debugger listening on ws://127.0.0.1:54651/3215a22e-09d3-4951-bd7c-11a8d2b1955b
04:46:16     [err] For help, see: https://nodejs.org/en/docs/inspector
04:46:16     [err] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [out] {}
04:46:16     [out] 
04:46:16     [err] Debugger attached.
04:46:16     [err] Waiting for the debugger to disconnect...
04:46:16     [err] 
04:46:16     { AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
04:46:16     + expected - actual
04:46:16     
04:46:16     - 3221225477
04:46:16     + 0
04:46:16         at runTests (c:\workspace\node-test-binary-windows\test\sequential\test-inspector-stop-profile-after-done.js:28:10)
04:46:16         at process._tickCallback (internal/process/next_tick.js:68:7)
04:46:16       generatedMessage: true,
04:46:16       name: 'AssertionError [ERR_ASSERTION]',
04:46:16       code: 'ERR_ASSERTION',
04:46:16       actual: 3221225477,
04:46:16       expected: 0,
04:46:16       operator: 'strictEqual' }
04:46:16     1
04:46:16   ...

And the second is definitely unrelated:

04:44:45 not ok 494 parallel/test-worker-relative-path-double-dot
04:44:45   ---
04:44:45   duration_ms: 120.98
04:44:45   severity: fail
04:44:45   exitcode: 1
04:44:45   stack: |-
04:44:45     timeout

@Trott I think this can land.

Trott · 2018-07-05T18:53:21Z

Here's a re-run of Windows (the only platform that failed):

https://ci.nodejs.org/job/node-test-commit-windows-fanned/19126/

mcollina · 2018-07-05T19:10:02Z

test-https-foafssl.js failed. I've hit resume, but probably I've caused more harm than good.

mcollina · 2018-07-05T19:11:55Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15746/ (maybe with more luck)

mcollina · 2018-07-06T22:05:13Z

Landed in 19795d8

Adds require('inspector').console, mapping it to the original global.console of V8. This enables applications to send messages to the inspector console programmatically. Fixes: #21651 PR-URL: #21659 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

mcollina · 2018-07-09T15:45:17Z

@targos this should land after #21378 or a backport PR being created to work around it.

Adds require('inspector').console, mapping it to the original global.console of V8. This enables applications to send messages to the inspector console programmatically. Fixes: #21651 PR-URL: #21659 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * inspector: * Expose the original console API in `require('inspector').console`. (#21659) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

nodejs-github-bot added the inspector Issues and PRs related to the V8 inspector protocol label Jul 4, 2018

mcollina added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 4, 2018

devsnek approved these changes Jul 4, 2018

View reviewed changes

targos approved these changes Jul 4, 2018

View reviewed changes

TimothyGu approved these changes Jul 4, 2018

View reviewed changes

TimothyGu reviewed Jul 4, 2018

View reviewed changes

mcollina force-pushed the add-inspector-console branch from 853355c to 4fce9da Compare July 5, 2018 07:22

mcollina added the notable-change PRs with changes that should be highlighted in changelogs. label Jul 5, 2018

inspector: expose original console

18fff38

Adds require('inspector').console, mapping it to the original global.console of V8. This enables applications to send messages to the inspector console programmatically. Fixes: nodejs#21651

mcollina force-pushed the add-inspector-console branch from 4fce9da to 18fff38 Compare July 5, 2018 08:01

TimothyGu approved these changes Jul 5, 2018

View reviewed changes

cjihrig approved these changes Jul 5, 2018

View reviewed changes

mcollina closed this Jul 6, 2018

mcollina deleted the add-inspector-console branch July 6, 2018 22:05

mcollina added the backport-requested-v10.x label Jul 9, 2018

targos mentioned this pull request Jul 10, 2018

doc: fix structure and formatting in inspector.md #21709

Closed

3 tasks

mcollina mentioned this pull request Jul 17, 2018

v10.7.0 proposal #21851

Merged

targos removed the backport-requested-v10.x label Jul 18, 2018

jdalton mentioned this pull request Aug 1, 2018

Migration from process.binding #22064

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inspector: expose original console #21659

inspector: expose original console #21659

mcollina commented Jul 4, 2018

nodejs-github-bot commented Jul 4, 2018

mcollina commented Jul 4, 2018

devsnek left a comment

TimothyGu left a comment

TimothyGu Jul 4, 2018

TimothyGu Jul 4, 2018

Trott commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 5, 2018

Trott commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 6, 2018

mcollina commented Jul 9, 2018

inspector: expose original console #21659

inspector: expose original console #21659

Conversation

mcollina commented Jul 4, 2018

Checklist

nodejs-github-bot commented Jul 4, 2018

mcollina commented Jul 4, 2018

devsnek left a comment

Choose a reason for hiding this comment

TimothyGu left a comment

Choose a reason for hiding this comment

TimothyGu Jul 4, 2018

Choose a reason for hiding this comment

TimothyGu Jul 4, 2018

Choose a reason for hiding this comment

Trott commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 5, 2018

Trott commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 5, 2018

mcollina commented Jul 6, 2018

mcollina commented Jul 9, 2018