Skip to content

Commit

Permalink
tty: do not end in an infinite warning recursion
Browse files Browse the repository at this point in the history
It was possible that this warning ends up in an infinite recursion.
The reason is that printing the warning triggered a color check and
that triggered another warning. Limiting it to a single warning
prevents this.

PR-URL: nodejs#31429
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
BridgeAR committed Feb 9, 2020
1 parent 0f6fed4 commit 63f10b9
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 11 deletions.
16 changes: 12 additions & 4 deletions lib/internal/tty.js
Expand Up @@ -73,18 +73,26 @@ const TERM_ENVS_REG_EXP = [
/^vt100/
];

let warned = false;
function warnOnDeactivatedColors(env) {
let name;
if (warned)
return;
let name = '';
if (env.NODE_DISABLE_COLORS !== undefined)
name = 'NODE_DISABLE_COLORS';
if (env.NO_COLOR !== undefined)
name = 'NO_COLOR';
if (env.NO_COLOR !== undefined) {
if (name !== '') {
name += "' and '";
}
name += 'NO_COLOR';
}

if (name !== undefined) {
if (name !== '') {
process.emitWarning(
`The '${name}' env is ignored due to the 'FORCE_COLOR' env being set.`,
'Warning'
);
warned = true;
}
}

Expand Down
8 changes: 8 additions & 0 deletions test/pseudo-tty/test-tty-color-support-warning-2.js
@@ -0,0 +1,8 @@
'use strict';

require('../common');

process.env.NODE_DISABLE_COLORS = '1';
process.env.FORCE_COLOR = '3';

console.log();
2 changes: 2 additions & 0 deletions test/pseudo-tty/test-tty-color-support-warning-2.out
@@ -0,0 +1,2 @@

(node:*) Warning: The 'NODE_DISABLE_COLORS' env is ignored due to the 'FORCE_COLOR' env being set.
9 changes: 9 additions & 0 deletions test/pseudo-tty/test-tty-color-support-warning.js
@@ -0,0 +1,9 @@
'use strict';

require('../common');

process.env.NO_COLOR = '1';
process.env.NODE_DISABLE_COLORS = '1';
process.env.FORCE_COLOR = '3';

console.log();
2 changes: 2 additions & 0 deletions test/pseudo-tty/test-tty-color-support-warning.out
@@ -0,0 +1,2 @@

(node:*) Warning: The 'NODE_DISABLE_COLORS' and 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set.
7 changes: 0 additions & 7 deletions test/pseudo-tty/test-tty-color-support.out
@@ -1,8 +1 @@
(node:*) Warning: The 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NODE_DISABLE_COLORS' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NODE_DISABLE_COLORS' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NODE_DISABLE_COLORS' env is ignored due to the 'FORCE_COLOR' env being set.
(node:*) Warning: The 'NODE_DISABLE_COLORS' env is ignored due to the 'FORCE_COLOR' env being set.

0 comments on commit 63f10b9

Please sign in to comment.