console: add support for console.debug method #17033

Tiriel · 2017-11-14T23:04:58Z

Adds the console.debug() method, alias for console.log()
This method is exposed by V8 and was only available in inspector until now.
Also adds matching test.

Refs: #17004 (review)

Checklist

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

Affected core subsystem(s)

console, tests

Fishrock123 · 2017-11-14T23:09:07Z

Does the spec provide provisions for debug to print extra info? If so, maybe we should?

Tiriel · 2017-11-14T23:12:01Z

@Fishrock123 I was wondering the same thing, but the only material I found (with my limited abilities) is the Chrome DevTools reference, which only states that console.debug() is identical to console.log() : https://developers.google.com/web/tools/chrome-devtools/console/console-reference#consoledebugobject_object
Same thing for the MDN: https://developer.mozilla.org/en-US/docs/Web/API/Console

TimothyGu · 2017-11-14T23:42:20Z

lib/console.js

@@ -134,6 +134,9 @@ Console.prototype.log = function log(...args) {
 };


+Console.prototype.debug = Console.prototype.log;


Per MDN,

Starting with Chromium 58 this method only appears in Chromium browser consoles when level "Verbose" is selected.

Thus for consistency, I don't think we should show debug logs by default, per default. Instead we could perhaps only display debug logs when an environment variable is specified. @nodejs/collaborators thoughts?

I like having debug be conditional upon some flag. Having it just be an alias doesn't seem that appealing.

Firefox and I believe every other major browser do not require a flag for console.debug() to display. I'm not sure we should follow Chrome's practice on this one. (I'm not sure we shouldn't either. I guess it kinda makes sense in Node.js for debug() to be conditional. So, maybe?)

I think that choosing t to follow Chrome on this feature would mean getting a bit too much opinionated, and I don't think we should. As you said, no other browser makes this choice.
The choice to restrain visibility of the debug should be made by the users IMHO, and Node should only provide the method.

I’m good with this. If they want to, anybody can override this method in their App, globally, and possibly depending on environment variables…

Tiriel · 2017-11-15T00:25:30Z

I know I'm not a collaborator, but here are my two cents on the matter:

I'm not sure that MDN is quite up to date on this note. The Google reference doesn't say a word about verbosity levels, and since it's supposed to be a Chromium browsers specificity, it seems reasonable to assume they would mention it.

Apart from this note, both sources mention specifically that console.debug() is just an alias, identical to console.log(). Any consideration of verbosity should then be dealt with in userland IMHO.

TimothyGu · 2017-11-15T00:28:35Z

@Tiriel

I know I'm not a collaborator

Apologies, I didn't mean to exclude you from the conversation, rather to try to get more people to look at this.

I think the Google reference is the out-of-date one here, as the "verbose" mode is not enabled by default.

Tiriel · 2017-11-15T00:38:32Z

@TimothyGu no problem, I didn't meant it to sound agressive either. I do acknowledge I'm not collaborator and may not have sufficient knowledge to be relevant.

And my bad for the verbosity levels, it appears you are right. But if this is a Chromium specificity indeed, I still think we shouldn't restrain it ourselves. V8 exposes the method anyway.

tniessen · 2017-11-15T09:37:36Z

I don't have a strong opinion on this. Chrome seems to have changed console.debug from being an alias for console.log to being a slightly more convenient logging facility. MDN discourages using console.debug, and most people tend to stay away from console.* in Node.js production environments anyway. However, as the current behavior (as of node 9) seems to be to suppress all messages, I would prefer to conditionally display console.debug messages instead of making it an alias. (I probably would not advise to use that function though.)

Tiriel · 2017-11-15T14:47:41Z

Just quoting @Trott on #17004 (wrong PR?)

I guess it's also worth noting that the WhatWG Living Standard says that debug() should do the same thing as log(). So that would seem to be another data point for not hiding its functionality behind a flag the way Chrome (but only Chrome) does.

I do agree on that, I hadn't thought about the standard but it does make a point.

TimothyGu · 2017-11-15T21:57:50Z

@Tiriel

But if this is a Chromium specificity indeed, I still think we shouldn't restrain it ourselves. V8 exposes the method anyway.

The only reason why V8 exposes this method is so that we can provide interop between DevTools' console.debug and the Node.js stdout/stderr console.debug. Thus, to me, it makes more than enough sense to follow the DevTools behavior -- its default behavior at least.

@tniessen

However, as the current behavior (as of node 9) seems to be to suppress all messages, I would prefer to conditionally display console.debug messages instead of making it an alias.

I would say that is a very good point in favor of suppressing debug messages by default, too.

@Trott

I guess it's also worth noting that the WhatWG Living Standard says that debug() should do the same thing as log(). So that would seem to be another data point for not hiding its functionality behind a flag the way Chrome (but only Chrome) does.

That makes sense... except the Living Standard says debug() is different from log(). debug() uses Logger("debug", data), while log() uses Logger("log", data), where Logger() is an internal operation that takes a priority/verbosity as the first parameter.

Tiriel · 2017-11-15T22:32:04Z

On the other hand, this does get us opinionated, replicating the behavior of a single browser where all the others don't make any difference. I have to admit I fail to see why we should privilege this behavior.

And as for the Living Standard, they do specify the verbosity level. But in that case we need to rethink all the methods in the console, for example console.error() is an alias of console.warn(). What the standard implies IMHO is that all the logging methods should be treated the same. Would it be only for the sake of consistency, debug should then be treated like log and info.

TimothyGu

@Tiriel has convinced me this is the way to go. However, docs are needed for this function.

Adds the console.debug() method, alias for console.log() This method is exposed by V8 and was only available in inspector until now. Also adds matching test. Refs: nodejs#17004 (review)

Removed this description from PR nodejs#17004 and included in this PR following review requesting changes. Fixes: nodejs#17033 (review) Refs: nodejs#17004 (review)

Tiriel · 2017-11-16T04:09:27Z

@TimothyGu Thanks! Very fair point. I've removed the doc for debug from #17004 to include it in this PR.

TimothyGu

LGTM!

Trott · 2017-11-17T00:40:03Z

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

Tiriel · 2017-11-17T08:12:42Z

Anything I can do to make the CI go green?

Trott · 2017-11-17T08:19:19Z

@Tiriel Fortunately for this PR (but unfortunately for Node.js) the failures are known problematic tests unrelated to this change. CI looks good.

Adds the console.debug() method, alias for console.log(). This method is exposed by V8 and was only available in inspector until now. Also adds matching test and documentation. PR-URL: #17033 Refs: #17004 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

tniessen · 2017-11-18T11:44:18Z

Landed in 45e6642, thank you! 😃

vsemozhetbyt · 2017-11-18T12:50:36Z

@Tiriel Thank you for the follow-up!

As seen in PR nodejs#17033, console.debug() is being fully implemented. Removing its description from here to include it in the other PR Fixes: nodejs#16755 Refs: nodejs#17033 (comment)

Following comment in PR nodejs#17033, and since this block of doc was first written in this PR, changing the implementation version number for console.debug(). Making it v9.2.x for now, waiting for next release. Fixes: nodejs#17033 (comment)

Adds the console.debug() method, alias for console.log(). This method is exposed by V8 and was only available in inspector until now. Also adds matching test and documentation. PR-URL: #17033 Refs: #17004 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * async\_hooks: - add trace events to async_hooks (Andreas Madsen) #15538 - add provider types for net server (Andreas Madsen) #17157 * console: - console.debug can now be used outside of the inspector (Benjamin Zaslavsky) #17033 * deps: - upgrade libuv to 1.18.0 (cjihrig) #17282 - patch V8 to 6.2.414.46 (Myles Borins) #17206 * module: - module.builtinModules will return a list of built in modules (Jon Moss) #16386 * n-api: - add helper for addons to get the event loop (Anna Henningsen) #17109 * process: - process.setUncaughtExceptionCaptureCallback can now be used to customize behavior for `--abort-on-uncaught-exception` (Anna Henningsen) #17159 - A signal handler is now able to receive the signal code that triggered the handler. (Robert Rossmann) #15606 * src: - embedders can now use Node::CreatePlatform to create an instance of NodePlatform (Cheng Zhao) #16981 * stream: - writable.writableHighWaterMark and readable.readableHighWaterMark will return the values the stream object was instantiated with (Calvin Metcalf) #12860 * **Added new collaborators** * [maclover7](https://github.com/maclover7) Jon Moss * [guybedford](https://github.com/guybedford) Guy Bedford * [hashseed](https://github.com/hashseed) Yang Guo PR-URL: Coming Soon

Notable changes: * async\_hooks: - add trace events to async_hooks (Andreas Madsen) #15538 - add provider types for net server (Andreas Madsen) #17157 * console: - console.debug can now be used outside of the inspector (Benjamin Zaslavsky) #17033 * deps: - upgrade libuv to 1.18.0 (cjihrig) #17282 - patch V8 to 6.2.414.46 (Myles Borins) #17206 * module: - module.builtinModules will return a list of built in modules (Jon Moss) #16386 * n-api: - add helper for addons to get the event loop (Anna Henningsen) #17109 * process: - process.setUncaughtExceptionCaptureCallback can now be used to customize behavior for `--abort-on-uncaught-exception` (Anna Henningsen) #17159 - A signal handler is now able to receive the signal code that triggered the handler. (Robert Rossmann) #15606 * src: - embedders can now use Node::CreatePlatform to create an instance of NodePlatform (Cheng Zhao) #16981 * stream: - writable.writableHighWaterMark and readable.readableHighWaterMark will return the values the stream object was instantiated with (Calvin Metcalf) #12860 * **Added new collaborators** * [maclover7](https://github.com/maclover7) Jon Moss * [guybedford](https://github.com/guybedford) Guy Bedford * [hashseed](https://github.com/hashseed) Yang Guo PR-URL: #17631

MylesBorins · 2017-12-19T20:17:40Z

We are deciding to label this semver-minor for lts

Adds the console.debug() method, alias for console.log(). This method is exposed by V8 and was only available in inspector until now. Also adds matching test and documentation. PR-URL: #17033 Refs: #17004 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * deps: * update V8 to 6.2.414.46 (Michaël Zasso) [#16413](#16413) * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [#16413](#16413) * upgrade libuv to 1.19.1 (cjihrig) [#18260](#18260) * re land npm 5.6.0 (Myles Borins) [#18625](#18625) * ICU 60 bump (Steven R. Loomis) [#16876](#16876) * crypto: * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [#16130](#16130) * warn on invalid authentication tag length (Tobias Nießen) [#17566](#17566) * async_hooks: * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [#18004](#18004) * use typed array stack as fast path (Anna Henningsen) [#17780](#17780) * use scope for defaultTriggerAsyncId (Andreas Madsen) [#17273](#17273) * separate missing from default context (Andreas Madsen) [#17273](#17273) * rename initTriggerId (Andreas Madsen) [#17273](#17273) * deprecate undocumented API (Andreas Madsen) [#16972](#16972) * add destroy event for gced AsyncResources (Sebastian Mayr) [#16998](#16998) * add trace events to async_hooks (Andreas Madsen) [#15538](#15538) * set HTTPParser trigger to socket (Andreas Madsen) [#18003](#18003) * add provider types for net server (Andreas Madsen) [#17157](#17157) * n-api: * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109) * cli: * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [#16495](#16495) * console: * add support for console.debug (Benjamin Zaslavsky) [#17033](#17033) * module: * add builtinModules (Jon Moss) [#16386](#16386) * replace default paths in require.resolve() (cjihrig) [#17113](#17113) * src: * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109) * add process.ppid (cjihrig) [#16839](#16839) * http: * support generic `Duplex` streams (Anna Henningsen) [#16267](#16267) * add rawPacket in err of `clientError` event (XadillaX) [#17672](#17672) * better support for IPv6 addresses (Mattias Holmlund) [#14772](#14772) * net: * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](#17662) * process: * fix reading zero-length env vars on win32 (Anna Henningsen) [#18463](#18463) * tls: * unconsume stream on destroy (Anna Henningsen) [#17478](#17478) * process: * improve unhandled rejection message (Madara Uchiha) [#17158](#17158) * stream: * remove usage of *State.highWaterMark (Calvin Metcalf) [#12860](#12860) * trace_events: * add executionAsyncId to init events (Andreas Madsen) [#17196](#17196) PR-URL: #18336

Notable changes: * deps: * update V8 to 6.2.414.46 (Michaël Zasso) [nodejs#16413](nodejs#16413) * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [nodejs#16413](nodejs#16413) * upgrade libuv to 1.19.1 (cjihrig) [nodejs#18260](nodejs#18260) * re land npm 5.6.0 (Myles Borins) [nodejs#18625](nodejs#18625) * ICU 60 bump (Steven R. Loomis) [nodejs#16876](nodejs#16876) * crypto: * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [nodejs#16130](nodejs#16130) * warn on invalid authentication tag length (Tobias Nießen) [nodejs#17566](nodejs#17566) * async_hooks: * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [nodejs#18004](nodejs#18004) * use typed array stack as fast path (Anna Henningsen) [nodejs#17780](nodejs#17780) * use scope for defaultTriggerAsyncId (Andreas Madsen) [nodejs#17273](nodejs#17273) * separate missing from default context (Andreas Madsen) [nodejs#17273](nodejs#17273) * rename initTriggerId (Andreas Madsen) [nodejs#17273](nodejs#17273) * deprecate undocumented API (Andreas Madsen) [nodejs#16972](nodejs#16972) * add destroy event for gced AsyncResources (Sebastian Mayr) [nodejs#16998](nodejs#16998) * add trace events to async_hooks (Andreas Madsen) [nodejs#15538](nodejs#15538) * set HTTPParser trigger to socket (Andreas Madsen) [nodejs#18003](nodejs#18003) * add provider types for net server (Andreas Madsen) [nodejs#17157](nodejs#17157) * n-api: * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109) * cli: * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [nodejs#16495](nodejs#16495) * console: * add support for console.debug (Benjamin Zaslavsky) [nodejs#17033](nodejs#17033) * module: * add builtinModules (Jon Moss) [nodejs#16386](nodejs#16386) * replace default paths in require.resolve() (cjihrig) [nodejs#17113](nodejs#17113) * src: * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109) * add process.ppid (cjihrig) [nodejs#16839](nodejs#16839) * http: * support generic `Duplex` streams (Anna Henningsen) [nodejs#16267](nodejs#16267) * add rawPacket in err of `clientError` event (XadillaX) [nodejs#17672](nodejs#17672) * better support for IPv6 addresses (Mattias Holmlund) [nodejs#14772](nodejs#14772) * net: * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [nodejs#17662](nodejs#17662) * process: * fix reading zero-length env vars on win32 (Anna Henningsen) [nodejs#18463](nodejs#18463) * tls: * unconsume stream on destroy (Anna Henningsen) [nodejs#17478](nodejs#17478) * process: * improve unhandled rejection message (Madara Uchiha) [nodejs#17158](nodejs#17158) * stream: * remove usage of *State.highWaterMark (Calvin Metcalf) [nodejs#12860](nodejs#12860) * trace_events: * add executionAsyncId to init events (Andreas Madsen) [nodejs#17196](nodejs#17196) PR-URL: nodejs#18336

nodejs-github-bot added the console Issues and PRs related to the console subsystem. label Nov 14, 2017

cjihrig approved these changes Nov 14, 2017

View reviewed changes

TimothyGu reviewed Nov 14, 2017

View reviewed changes

XadillaX approved these changes Nov 15, 2017

View reviewed changes

targos approved these changes Nov 15, 2017

View reviewed changes

addaleax approved these changes Nov 15, 2017

View reviewed changes

Tiriel mentioned this pull request Nov 15, 2017

doc: add description for inspector-only console methods. #17004

Closed

3 tasks

TimothyGu suggested changes Nov 15, 2017

View reviewed changes

Tiriel added 2 commits November 16, 2017 05:06

console: add support for console.debug method

ec3638d

Adds the console.debug() method, alias for console.log() This method is exposed by V8 and was only available in inspector until now. Also adds matching test. Refs: nodejs#17004 (review)

doc: adding description for console.debug

384a60e

Removed this description from PR nodejs#17004 and included in this PR following review requesting changes. Fixes: nodejs#17033 (review) Refs: nodejs#17004 (review)

Tiriel force-pushed the console-add-debug branch from 25e9034 to 384a60e Compare November 16, 2017 04:06

TimothyGu approved these changes Nov 16, 2017

View reviewed changes

jasnell approved these changes Nov 16, 2017

View reviewed changes

tniessen closed this Nov 18, 2017

Tiriel mentioned this pull request Nov 18, 2017

console: implement console.table and console.dirxml #17128

Closed

MylesBorins mentioned this pull request Dec 12, 2017

v9.3.0 proposal #17631

Merged

MylesBorins added the semver-minor PRs that contain new features and should be released in the next minor version. label Dec 19, 2017

MylesBorins added baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v6.x labels Dec 19, 2017

Tiriel mentioned this pull request Dec 20, 2017

console: add dirxml method #17152

Closed

4 tasks

MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

console: add support for console.debug method #17033

console: add support for console.debug method #17033

Tiriel commented Nov 14, 2017

Fishrock123 commented Nov 14, 2017

Tiriel commented Nov 14, 2017

TimothyGu Nov 14, 2017

apapirovski Nov 15, 2017

Trott Nov 15, 2017 •

edited

Loading

Tiriel Nov 15, 2017

addaleax Nov 15, 2017

Tiriel commented Nov 15, 2017

TimothyGu commented Nov 15, 2017 •

edited

Loading

Tiriel commented Nov 15, 2017

tniessen commented Nov 15, 2017

Tiriel commented Nov 15, 2017 •

edited

Loading

TimothyGu commented Nov 15, 2017

Tiriel commented Nov 15, 2017

TimothyGu left a comment

Tiriel commented Nov 16, 2017 •

edited

Loading

TimothyGu left a comment

Trott commented Nov 17, 2017

Tiriel commented Nov 17, 2017

Trott commented Nov 17, 2017

tniessen commented Nov 18, 2017

vsemozhetbyt commented Nov 18, 2017

MylesBorins commented Dec 19, 2017

		@@ -134,6 +134,9 @@ Console.prototype.log = function log(...args) {
		};


		Console.prototype.debug = Console.prototype.log;

console: add support for console.debug method #17033

console: add support for console.debug method #17033

Conversation

Tiriel commented Nov 14, 2017

Checklist

Affected core subsystem(s)

Fishrock123 commented Nov 14, 2017

Tiriel commented Nov 14, 2017

TimothyGu Nov 14, 2017

Choose a reason for hiding this comment

apapirovski Nov 15, 2017

Choose a reason for hiding this comment

Trott Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

Tiriel Nov 15, 2017

Choose a reason for hiding this comment

addaleax Nov 15, 2017

Choose a reason for hiding this comment

Tiriel commented Nov 15, 2017

TimothyGu commented Nov 15, 2017 • edited Loading

Tiriel commented Nov 15, 2017

tniessen commented Nov 15, 2017

Tiriel commented Nov 15, 2017 • edited Loading

TimothyGu commented Nov 15, 2017

Tiriel commented Nov 15, 2017

TimothyGu left a comment

Choose a reason for hiding this comment

Tiriel commented Nov 16, 2017 • edited Loading

TimothyGu left a comment

Choose a reason for hiding this comment

Trott commented Nov 17, 2017

Tiriel commented Nov 17, 2017

Trott commented Nov 17, 2017

tniessen commented Nov 18, 2017

vsemozhetbyt commented Nov 18, 2017

MylesBorins commented Dec 19, 2017

Trott Nov 15, 2017 •

edited

Loading

TimothyGu commented Nov 15, 2017 •

edited

Loading

Tiriel commented Nov 15, 2017 •

edited

Loading

Tiriel commented Nov 16, 2017 •

edited

Loading