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: #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 authored and codebytere committed Feb 17, 2020
1 parent 58f17c0 commit be55f3e
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 be55f3e

Please sign in to comment.