From d40be9cbf254f78d2d125e79f815ccd39df32158 Mon Sep 17 00:00:00 2001
From: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Date: Fri, 6 Dec 2019 17:37:10 +0100
Subject: [PATCH] fix: use util.inspect for more reliable logging (#37880)

The tests are probably overkill, but this way we will know if
util.inspect changes dramatically.
---
 client/src/client/workers/test-evaluator.js   | 10 +---
 .../src/templates/Challenges/utils/frame.js   |  3 +-
 client/src/utils/format.js                    |  7 +++
 client/src/utils/format.test.js               | 56 +++++++++++++++++++
 4 files changed, 67 insertions(+), 9 deletions(-)
 create mode 100644 client/src/utils/format.js
 create mode 100644 client/src/utils/format.test.js

diff --git a/client/src/client/workers/test-evaluator.js b/client/src/client/workers/test-evaluator.js
index ea928c25335..5869047b52f 100644
--- a/client/src/client/workers/test-evaluator.js
+++ b/client/src/client/workers/test-evaluator.js
@@ -1,6 +1,7 @@
 import chai from 'chai';
 import '@babel/polyfill';
 import __toString from 'lodash/toString';
+import { format as __format } from '../../utils/format';
 
 const __utils = (() => {
   const MAX_LOGS_SIZE = 64 * 1024;
@@ -16,16 +17,9 @@ const __utils = (() => {
     }
   }
 
-  function replacer(key, value) {
-    if (Number.isNaN(value)) {
-      return 'NaN';
-    }
-    return value;
-  }
-
   const oldLog = self.console.log.bind(self.console);
   function proxyLog(...args) {
-    logs.push(args.map(arg => '' + JSON.stringify(arg, replacer)).join(' '));
+    logs.push(args.map(arg => __format(arg)).join(' '));
     if (logs.join('\n').length > MAX_LOGS_SIZE) {
       flushLogs();
     }
diff --git a/client/src/templates/Challenges/utils/frame.js b/client/src/templates/Challenges/utils/frame.js
index cf1cd9b0407..952921d0f95 100644
--- a/client/src/templates/Challenges/utils/frame.js
+++ b/client/src/templates/Challenges/utils/frame.js
@@ -1,4 +1,5 @@
 import { toString, flow } from 'lodash';
+import { format } from '../../../utils/format';
 
 // we use two different frames to make them all essentially pure functions
 // main iframe is responsible rendering the preview and is where we proxy the
@@ -81,7 +82,7 @@ const mountFrame = document => ({ element, ...rest }) => {
 const buildProxyConsole = proxyLogger => ctx => {
   const oldLog = ctx.window.console.log.bind(ctx.window.console);
   ctx.window.console.log = function proxyConsole(...args) {
-    proxyLogger(args.map(arg => '' + JSON.stringify(arg)).join(' '));
+    proxyLogger(args.map(arg => format(arg)).join(' '));
     return oldLog(...args);
   };
   return ctx;
diff --git a/client/src/utils/format.js b/client/src/utils/format.js
new file mode 100644
index 00000000000..616af68386e
--- /dev/null
+++ b/client/src/utils/format.js
@@ -0,0 +1,7 @@
+import { inspect } from 'util';
+
+export function format(x) {
+  // we're trying to mimic console.log, so we avoid wrapping strings in quotes:
+  if (typeof x === 'string') return x;
+  return inspect(x);
+}
diff --git a/client/src/utils/format.test.js b/client/src/utils/format.test.js
new file mode 100644
index 00000000000..05d38172577
--- /dev/null
+++ b/client/src/utils/format.test.js
@@ -0,0 +1,56 @@
+/* global expect BigInt */
+
+const { format } = require('./format');
+
+/* eslint-disable no-unused-vars */
+function simpleFun() {
+  var x = 'y';
+}
+/* eslint-enable no-unused-vars */
+
+/* format uses util.inspect to do almost everything, the tests are just there
+to warn us if util.inspect ever changes */
+describe('format', () => {
+  it('returns a string', () => {
+    expect(typeof format('')).toBe('string');
+    expect(typeof format({})).toBe('string');
+    expect(typeof format([])).toBe('string');
+  });
+  it('does not modify strings', () => {
+    expect(format('')).toBe('');
+    expect(format('abcde')).toBe('abcde');
+    expect(format('Case Sensitive')).toBe('Case Sensitive');
+  });
+  it('formats shallow objects nicely', () => {
+    expect(format({})).toBe('{}');
+    expect(format({ a: 'one', b: 'two' })).toBe(`{ a: 'one', b: 'two' }`);
+  });
+  it('formats functions the same way as console.log', () => {
+    expect(format(simpleFun)).toBe('[Function: simpleFun]');
+  });
+  it('recurses into arrays', () => {
+    const objsInArr = [{ a: 'one' }, 'b', simpleFun];
+    expect(format(objsInArr)).toBe(
+      `[ { a: 'one' }, 'b', [Function: simpleFun] ]`
+    );
+  });
+  it('handles all primitive values', () => {
+    const primitives = [
+      'str',
+      57,
+      BigInt(10),
+      true,
+      false,
+      null,
+      // eslint-disable-next-line no-undefined
+      undefined,
+      Symbol('Sym')
+    ];
+    expect(format(primitives)).toBe(
+      `[ 'str', 57, 10n, true, false, null, undefined, Symbol(Sym) ]`
+    );
+  });
+  it(`outputs NaN as 'NaN'`, () => {
+    expect(format(NaN)).toBe('NaN');
+  });
+});