lib: support FORCE_COLOR for non TTY streams

MoLow · danielleadams · commit c46b31f3bfc0 · 2023-07-11T22:02:04.000-04:00
PR-URL: #48034 Backport-PR-URL: #48684 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js
@@ -80,6 +80,12 @@ const kMaxGroupIndentation = 1000;
 // Lazy loaded for startup performance.
 let cliTable;
 
+let utilColors;
+function lazyUtilColors() {
+ utilColors ??= require('internal/util/colors');
+ return utilColors;
+}
+
 // Track amount of indentation required via `console.group()`.
 const kGroupIndent = Symbol('kGroupIndent');
 const kGroupIndentationWidth = Symbol('kGroupIndentWidth');
@@ -96,7 +102,6 @@ const kUseStdout = Symbol('kUseStdout');
 const kUseStderr = Symbol('kUseStderr');
 
 const optionsMap = new SafeWeakMap();
-
 function Console(options /* or: stdout, stderr, ignoreErrors = true */) {
 // We have to test new.target here to see if this function is called
 // with new, because we need to define a custom instanceof to accommodate
@@ -315,9 +320,7 @@ ObjectDefineProperties(Console.prototype, {
 value: function(stream) {
 let color = this[kColorMode];
 if (color === 'auto') {
- color = stream.isTTY && (
- typeof stream.getColorDepth === 'function' ?
- stream.getColorDepth() > 2 : true);
+ color = lazyUtilColors().shouldColorize(stream);
 }
 
 const options = optionsMap.get(this);
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -190,6 +190,12 @@ function lazyInternalUtilInspect() {
 return internalUtilInspect;
 }
 
+let utilColors;
+function lazyUtilColors() {
+ utilColors ??= require('internal/util/colors');
+ return utilColors;
+}
+
 let buffer;
 function lazyBuffer() {
 buffer ??= require('buffer').Buffer;
@@ -795,10 +801,7 @@ const fatalExceptionStackEnhancers = {
 colors: defaultColors,
 },
 } = lazyInternalUtilInspect();
- const colors = useColors &&
- ((internalBinding('util').guessHandleType(2) === 'TTY' &&
- require('internal/tty').hasColors()) ||
- defaultColors);
+ const colors = useColors && (lazyUtilColors().shouldColorize(process.stderr) || defaultColors);
 try {
 return inspect(error, {
 colors,
diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js
@@ -14,11 +14,11 @@ const {
 const assert = require('assert');
 const Transform = require('internal/streams/transform');
 const { inspectWithNoCustomRetry } = require('internal/errors');
-const { green, blue, red, white, gray, hasColors } = require('internal/util/colors');
+const { green, blue, red, white, gray, shouldColorize } = require('internal/util/colors');
 const { kSubtestsFailed } = require('internal/test_runner/test');
 const { getCoverageReport } = require('internal/test_runner/utils');
 
-const inspectOptions = { __proto__: null, colors: hasColors, breakLength: Infinity };
+const inspectOptions = { __proto__: null, colors: shouldColorize(process.stdout), breakLength: Infinity };
 
 const colors = {
 '__proto__': null,
diff --git a/lib/internal/util/colors.js b/lib/internal/util/colors.js
@@ -1,5 +1,11 @@
 'use strict';
 
+let internalTTy;
+function lazyInternalTTY() {
+ internalTTy ??= require('internal/tty');
+ return internalTTy;
+}
+
 module.exports = {
 blue: '',
 green: '',
@@ -8,9 +14,17 @@ module.exports = {
 gray: '',
 clear: '',
 hasColors: false,
+ shouldColorize(stream) {
+ if (process.env.FORCE_COLOR !== undefined) {
+ return lazyInternalTTY().getColorDepth() > 2;
+ }
+ return stream?.isTTY && (
+ typeof stream.getColorDepth === 'function' ?
+ stream.getColorDepth() > 2 : true);
+ },
 refresh() {
 if (process.stderr.isTTY) {
- const hasColors = process.stderr.hasColors();
+ const hasColors = module.exports.shouldColorize(process.stderr);
 module.exports.blue = hasColors ? '\u001b[34m' : '';
 module.exports.green = hasColors ? '\u001b[32m' : '';
 module.exports.white = hasColors ? '\u001b[39m' : '';
diff --git a/lib/internal/util/debuglog.js b/lib/internal/util/debuglog.js
@@ -46,13 +46,19 @@ function emitWarningIfNeeded(set) {
 
 const noop = () => {};
 
+let utilColors;
+function lazyUtilColors() {
+ utilColors ??= require('internal/util/colors');
+ return utilColors;
+}
+
 function debuglogImpl(enabled, set) {
 if (debugImpls[set] === undefined) {
 if (enabled) {
 const pid = process.pid;
 emitWarningIfNeeded(set);
 debugImpls[set] = function debug(...args) {
- const colors = process.stderr.hasColors && process.stderr.hasColors();
+ const colors = lazyUtilColors().shouldColorize(process.stderr);
 const msg = formatWithOptions({ colors }, ...args);
 const coloredPID = inspect(pid, { colors });
 process.stderr.write(format('%s %s: %s\n', set, coloredPID, msg));
diff --git a/lib/repl.js b/lib/repl.js
@@ -122,6 +122,7 @@ const {
 commonPrefix,
 } = require('internal/readline/utils');
 const { Console } = require('console');
+const { shouldColorize } = require('internal/util/colors');
 const CJSModule = require('internal/modules/cjs/loader').Module;
 let _builtinLibs = ArrayPrototypeFilter(
 CJSModule.builtinModules,
@@ -272,11 +273,7 @@ function REPLServer(prompt,
 
 if (options.terminal && options.useColors === undefined) {
 // If possible, check if stdout supports colors or not.
- if (options.output.hasColors) {
- options.useColors = options.output.hasColors();
- } else if (process.env.NODE_DISABLE_COLORS === undefined) {
- options.useColors = true;
- }
+ options.useColors = shouldColorize(options.output) || process.env.NODE_DISABLE_COLORS === undefined;
 }
 
 // TODO(devsnek): Add a test case for custom eval functions.
diff --git a/test/common/assertSnapshot.js b/test/common/assertSnapshot.js
@@ -54,15 +54,15 @@ async function assertSnapshot(actual, filename = process.argv[1]) {
 * @param {boolean} [options.tty] - whether to spawn the process in a pseudo-tty
 * @returns {Promise<void>}
 */
-async function spawnAndAssert(filename, transform = (x) => x, { tty = false } = {}) {
+async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...options } = {}) {
 if (tty && common.isWindows) {
 test({ skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.' });
 return;
 }
 const flags = common.parseTestFlags(filename);
 const executable = tty ? 'tools/pseudo-tty.py' : process.execPath;
 const args = tty ? [process.execPath, ...flags, filename] : [...flags, filename];
- const { stdout, stderr } = await common.spawnPromisified(executable, args);
+ const { stdout, stderr } = await common.spawnPromisified(executable, args, options);
 await assertSnapshot(transform(`${stdout}${stderr}`), filename);
 }
 
diff --git a/test/fixtures/console/force_colors.js b/test/fixtures/console/force_colors.js
@@ -0,0 +1,5 @@
+'use strict';
+
+require('../../common');
+
+console.log(123, 'foo', { bar: 'baz' });
diff --git a/test/fixtures/console/force_colors.snapshot b/test/fixtures/console/force_colors.snapshot
@@ -0,0 +1 @@
+[33m123[39m foo { bar: [32m'baz'[39m }
diff --git a/test/fixtures/errors/force_colors.js b/test/fixtures/errors/force_colors.js
@@ -0,0 +1 @@
+throw new Error('Should include grayed stack trace')
diff --git a/test/fixtures/errors/force_colors.snapshot b/test/fixtures/errors/force_colors.snapshot
@@ -0,0 +1,14 @@
+*force_colors.js:1
+throw new Error('Should include grayed stack trace')
+^
+
+Error: Should include grayed stack trace
+ at Object.<anonymous> [90m(/[39mtest*force_colors.js:1:7[90m)[39m
+[90m at Module._compile (node:internal*modules*cjs*loader:1256:14)[39m
+[90m at Module._extensions..js (node:internal*modules*cjs*loader:1310:10)[39m
+[90m at Module.load (node:internal*modules*cjs*loader:1119:32)[39m
+[90m at Module._load (node:internal*modules*cjs*loader:960:12)[39m
+[90m at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:81:12)[39m
+[90m at node:internal*main*run_main_module:23:47[39m
+
+Node.js *
diff --git a/test/parallel/test-node-output-console.mjs b/test/parallel/test-node-output-console.mjs
@@ -3,6 +3,9 @@ import * as fixtures from '../common/fixtures.mjs';
 import * as snapshot from '../common/assertSnapshot.js';
 import { describe, it } from 'node:test';
 
+const skipForceColors =
+ process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
+ process.config.variables.node_shared_openssl;
 
 function replaceStackTrace(str) {
 return snapshot.replaceStackTrace(str, '$1at *$7\n');
@@ -22,12 +25,13 @@ describe('console output', { concurrency: true }, () => {
 transform: snapshot
 .transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, normalize)
 },
- ];
+ !skipForceColors ? { name: 'console/force_colors.js', env: { FORCE_COLOR: 1 } } : null,
+ ].filter(Boolean);
 const defaultTransform = snapshot
 .transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, replaceStackTrace);
- for (const { name, transform } of tests) {
+ for (const { name, transform, env } of tests) {
 it(name, async () => {
- await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
+ await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env });
 });
 }
 });
diff --git a/test/parallel/test-node-output-errors.mjs b/test/parallel/test-node-output-errors.mjs
@@ -1,8 +1,15 @@
-import '../common/index.mjs';
+import * as common from '../common/index.mjs';
 import * as fixtures from '../common/fixtures.mjs';
 import * as snapshot from '../common/assertSnapshot.js';
+import * as os from 'node:os';
 import { describe, it } from 'node:test';
 
+const skipForceColors =
+ process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
+ process.config.variables.node_shared_openssl ||
+ (common.isWindows && (Number(os.release().split('.')[0]) !== 10 || Number(os.release().split('.')[2]) < 14393)); // See https://github.com/nodejs/node/pull/33132
+
+
 function replaceNodeVersion(str) {
 return str.replaceAll(process.version, '*');
 }
@@ -43,10 +50,11 @@ describe('errors output', { concurrency: true }, () => {
 { name: 'errors/throw_in_line_with_tabs.js', transform: errTransform },
 { name: 'errors/throw_non_error.js', transform: errTransform },
 { name: 'errors/promise_always_throw_unhandled.js', transform: promiseTransform },
- ];
- for (const { name, transform } of tests) {
+ !skipForceColors ? { name: 'errors/force_colors.js', env: { FORCE_COLOR: 1 } } : null,
+ ].filter(Boolean);
+ for (const { name, transform, env } of tests) {
 it(name, async () => {
- await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
+ await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env });
 });
 }
 });
diff --git a/test/parallel/test-repl-envvars.js b/test/parallel/test-repl-envvars.js
@@ -4,6 +4,7 @@
 
 require('../common');
 const stream = require('stream');
+const { describe, test } = require('node:test');
 const REPL = require('internal/repl');
 const assert = require('assert');
 const inspect = require('util').inspect;
@@ -18,13 +19,21 @@ const tests = [
 env: { NODE_DISABLE_COLORS: '1' },
 expected: { terminal: true, useColors: false }
 },
+ {
+ env: { NODE_DISABLE_COLORS: '1', FORCE_COLOR: '1' },
+ expected: { terminal: true, useColors: true }
+ },
 {
 env: { NODE_NO_READLINE: '1' },
 expected: { terminal: false, useColors: false }
 },
 {
 env: { TERM: 'dumb' },
- expected: { terminal: true, useColors: false }
+ expected: { terminal: true, useColors: true }
+ },
+ {
+ env: { TERM: 'dumb', FORCE_COLOR: '1' },
+ expected: { terminal: true, useColors: true }
 },
 {
 env: { NODE_NO_READLINE: '1', NODE_DISABLE_COLORS: '1' },
@@ -56,20 +65,25 @@ function run(test) {
 
 Object.assign(process.env, env);
 
- REPL.createInternalRepl(process.env, opts, function(err, repl) {
- assert.ifError(err);
+ return new Promise((resolve) => {
+ REPL.createInternalRepl(process.env, opts, function(err, repl) {
+ assert.ifError(err);
 
- assert.strictEqual(repl.terminal, expected.terminal,
- `Expected ${inspect(expected)} with ${inspect(env)}`);
- assert.strictEqual(repl.useColors, expected.useColors,
- `Expected ${inspect(expected)} with ${inspect(env)}`);
- assert.strictEqual(repl.replMode, expected.replMode || REPL_MODE_SLOPPY,
- `Expected ${inspect(expected)} with ${inspect(env)}`);
- for (const key of Object.keys(env)) {
- delete process.env[key];
- }
- repl.close();
+ assert.strictEqual(repl.terminal, expected.terminal,
+ `Expected ${inspect(expected)} with ${inspect(env)}`);
+ assert.strictEqual(repl.useColors, expected.useColors,
+ `Expected ${inspect(expected)} with ${inspect(env)}`);
+ assert.strictEqual(repl.replMode, expected.replMode || REPL_MODE_SLOPPY,
+ `Expected ${inspect(expected)} with ${inspect(env)}`);
+ for (const key of Object.keys(env)) {
+ delete process.env[key];
+ }
+ repl.close();
+ resolve();
+ });
 });
 }
 
-tests.forEach(run);
+describe('REPL environment variables', { concurrency: 1 }, () => {
+ tests.forEach((testCase) => test(inspect(testCase.env), () => run(testCase)));
+});

-Original file line number
+Diff line change
 // Lazy loaded for startup performance.
 letcliTable;
 +letutilColors;
 +functionlazyUtilColors(){
 +utilColors??=require('internal/util/colors');
 +returnutilColors;
 +}
++
 // Track amount of indentation required via `console.group()`.
 constkGroupIndent=Symbol('kGroupIndent');
 constkGroupIndentationWidth=Symbol('kGroupIndentWidth');
 constkUseStderr=Symbol('kUseStderr');
 constoptionsMap=newSafeWeakMap();
+-
 functionConsole(options/* or: stdout, stderr, ignoreErrors = true */){
 // We have to test new.target here to see if this function is called
 // with new, because we need to define a custom instanceof to accommodate
 value: function(stream){
 letcolor=this[kColorMode];
 if(color==='auto'){
 -color=stream.isTTY&&(
 -typeofstream.getColorDepth==='function' ?
 -stream.getColorDepth()>2 : true);
 +color=lazyUtilColors().shouldColorize(stream);
+}
 constoptions=optionsMap.get(this);
-Original file line number
+Diff line change
 returninternalUtilInspect;
+}
 +letutilColors;
 +functionlazyUtilColors(){
 +utilColors??=require('internal/util/colors');
 +returnutilColors;
 +}
++
 letbuffer;
 functionlazyBuffer(){
 buffer??=require('buffer').Buffer;
 colors: defaultColors,
 },
 }=lazyInternalUtilInspect();
 -constcolors=useColors&&
 -((internalBinding('util').guessHandleType(2)==='TTY'&&
 -require('internal/tty').hasColors())||
 -defaultColors);
 +constcolors=useColors&&(lazyUtilColors().shouldColorize(process.stderr)||defaultColors);
 try{
 returninspect(error,{
  colors,
-Original file line number
+Diff line change
 constassert=require('assert');
 constTransform=require('internal/streams/transform');
 const{ inspectWithNoCustomRetry }=require('internal/errors');
 -const{ green, blue, red, white, gray,hasColors}=require('internal/util/colors');
 +const{ green, blue, red, white, gray,shouldColorize}=require('internal/util/colors');
 const{ kSubtestsFailed }=require('internal/test_runner/test');
 const{ getCoverageReport }=require('internal/test_runner/utils');
 -constinspectOptions={__proto__: null,colors: hasColors,breakLength: Infinity};
 +constinspectOptions={__proto__: null,colors: shouldColorize(process.stdout),breakLength: Infinity};
 constcolors={
 '__proto__': null,
-Original file line number
+Diff line change
@@ @@ -1,5 +1,11 @@ @@
 'use strict';
 +letinternalTTy;
 +functionlazyInternalTTY(){
 +internalTTy??=require('internal/tty');
 +returninternalTTy;
 +}
++
 module.exports={
 blue: '',
 green: '',
 gray: '',
 clear: '',
 hasColors: false,
 +shouldColorize(stream){
 +if(process.env.FORCE_COLOR!==undefined){
 +returnlazyInternalTTY().getColorDepth()>2;
 +}
 +returnstream?.isTTY&&(
 +typeofstream.getColorDepth==='function' ?
 +stream.getColorDepth()>2 : true);
 +},
 refresh(){
 if(process.stderr.isTTY){
 -consthasColors=process.stderr.hasColors();
 +consthasColors=module.exports.shouldColorize(process.stderr);
 module.exports.blue=hasColors ? '\u001b[34m' : '';
 module.exports.green=hasColors ? '\u001b[32m' : '';
 module.exports.white=hasColors ? '\u001b[39m' : '';
-Original file line number
+Diff line change
 constnoop=()=>{};
 +letutilColors;
 +functionlazyUtilColors(){
 +utilColors??=require('internal/util/colors');
 +returnutilColors;
 +}
++
 functiondebuglogImpl(enabled,set){
 if(debugImpls[set]===undefined){
 if(enabled){
 constpid=process.pid;
 emitWarningIfNeeded(set);
 debugImpls[set]=functiondebug(...args){
 -constcolors=process.stderr.hasColors&&process.stderr.hasColors();
 +constcolors=lazyUtilColors().shouldColorize(process.stderr);
 constmsg=formatWithOptions({ colors }, ...args);
 constcoloredPID=inspect(pid,{ colors });
 process.stderr.write(format('%s %s: %s\n',set,coloredPID,msg));
-Original file line number
+Diff line change
  commonPrefix,
 }=require('internal/readline/utils');
 const{ Console }=require('console');
 +const{ shouldColorize }=require('internal/util/colors');
 constCJSModule=require('internal/modules/cjs/loader').Module;
 let_builtinLibs=ArrayPrototypeFilter(
 CJSModule.builtinModules,
 if(options.terminal&&options.useColors===undefined){
 // If possible, check if stdout supports colors or not.
 -if(options.output.hasColors){
 -options.useColors=options.output.hasColors();
 -}elseif(process.env.NODE_DISABLE_COLORS===undefined){
 -options.useColors=true;
 -}
 +options.useColors=shouldColorize(options.output)||process.env.NODE_DISABLE_COLORS===undefined;
+}
 // TODO(devsnek): Add a test case for custom eval functions.
-Original file line number
+Diff line change
  * @param {boolean} [options.tty] - whether to spawn the process in a pseudo-tty
  * @returns {Promise<void>}
  */
 -asyncfunctionspawnAndAssert(filename,transform=(x)=>x,{ tty =false}={}){
 +asyncfunctionspawnAndAssert(filename,transform=(x)=>x,{ tty =false, ...options}={}){
 if(tty&&common.isWindows){
 test({skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.'});
 return;
+}
 constflags=common.parseTestFlags(filename);
 constexecutable=tty ? 'tools/pseudo-tty.py' : process.execPath;
 constargs=tty ? [process.execPath, ...flags,filename] : [...flags,filename];
 -const{ stdout, stderr }=awaitcommon.spawnPromisified(executable,args);
 +const{ stdout, stderr }=awaitcommon.spawnPromisified(executable,args,options);
 awaitassertSnapshot(transform(`${stdout}${stderr}`),filename);
+}
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+thrownewError('Should include grayed stack trace')`