lib: support FORCE_COLOR for non TTY streams

MoLow · MoLow · commit d7d3c0ae6246 · 2023-07-07T00:35:49.000+03:00
PR-URL: nodejs#48034 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 number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+throw new Error('Should include grayed stack trace')`