Handle incorrect keys to loggy (#1112)

* Handle incorrect keys to loggy

When calling Loggy.success with a non-existing key, the reference would not be found, and the call to path.basename in Loggy._log would fail (since its parameter was null). This commit:

- Catches any exceptions in _log, and displays an error (allowing to continue)
- Adds a testing mode to the logger, which is silent but executes logic, and throws an exception upon issues (only to be used in the CLI and lib tests)
- Fixes incorrect or missing keys in calls to the logger

* Remove failing test

packages/cli/src/models/network/NetworkController.ts

@@ -233,7 +233,7 @@ export default class NetworkController {
     Loggy.spin(__filename, '_uploadSolidityLib', `upload-solidity-lib${libName}`, `Uploading ${libName} library`);
     const libInstance = await this.project.setImplementation(libClass, libName);
     this.networkFile.addSolidityLib(libName, libInstance);
-    Loggy.succeed(`upload-solidity-lob${libName}`, `${libName} library uploaded`);
+    Loggy.succeed(`upload-solidity-lib${libName}`, `${libName} library uploaded`);
   }
 
   // Contract model

packages/cli/test/setup.js

@@ -14,7 +14,8 @@ useTestProjectFile();
 doNotInstallStdlib();
 ZWeb3.initialize(web3.currentProvider);
 setArtifactDefaults();
-Loggy.silent(true);
+Loggy.silent(false);
+Loggy.testing(true);
 
 require('chai')
   .use(require('chai-as-promised'))

packages/lib/src/project/ProxyAdminProject.ts

@@ -52,6 +52,7 @@ class BaseProxyAdminProject extends BaseSimpleProject {
       contract,
       contractParams,
     );
+    Loggy.spin(__filename, 'upgradeProxy', `action-proxy-${pAddress}`, `Upgrading instance at ${pAddress}`);
     await this.proxyAdmin.upgradeProxy(pAddress, implementationAddress, contract, initMethodName, initArgs);
     Loggy.succeed(`action-proxy-${pAddress}`, `Instance at ${pAddress} upgraded`);
     return contract.at(pAddress);

packages/lib/src/project/SimpleProject.ts

@@ -22,9 +22,10 @@ export default class SimpleProject extends BaseSimpleProject {
       contract,
       contractParams,
     );
+    Loggy.spin(__filename, 'upgradeProxy', `action-proxy-${pAddress}`, `Upgrading instance at ${pAddress}`);
     const proxy = Proxy.at(pAddress, this.txParams);
     await proxy.upgradeTo(implementationAddress, initCallData);
-    Loggy.succeed(`action-proxy-${implementationAddress}`, `Instance at ${pAddress} upgraded`);
+    Loggy.succeed(`action-proxy-${pAddress}`, `Instance at ${pAddress} upgraded`);
     return contract.at(proxyAddress);
   }
 

packages/lib/src/utils/Logger.ts

@@ -45,6 +45,7 @@ export const Loggy: { [key: string]: any } = {
   logs: {},
   isSilent: true,
   isVerbose: false,
+  isTesting: false,
 
   silent(value: boolean): void {
     this.isSilent = value;
@@ -54,6 +55,10 @@ export const Loggy: { [key: string]: any } = {
     this.isVerbose = value;
   },
 
+  testing(value: boolean): void {
+    this.isTesting = value;
+  },
+
   add(
     file: string,
     fnName: string,
@@ -113,18 +118,22 @@ export const Loggy: { [key: string]: any } = {
   },
 
   _log(reference: string): void {
-    if (this.isSilent) return;
-    const { file, fnName, text, spinnerAction, logLevel, logType } = this.logs[reference];
-    const color = this._getColorFor(logType);
-    if (this.isVerbose) {
-      const location = `${path.basename(file)}#${fnName}`;
-      const message = `[${new Date().toISOString()}@${location}] <${this._actionToText(spinnerAction)}> ${text}`;
-      const coloredMessage = color ? chalk.keyword(color)(message) : message;
-      console.error(coloredMessage);
-    } else if (logLevel === LogLevel.Normal) {
-      const options = color ? { text, status: spinnerAction, color } : { text, status: spinnerAction };
-
-      !spinners.pick(reference) ? spinners.add(reference, options) : spinners.update(reference, options);
+    try {
+      if (this.isSilent) return;
+      const { file, fnName, text, spinnerAction, logLevel, logType } = this.logs[reference];
+      const color = this._getColorFor(logType);
+      if (this.isVerbose || this.isTesting) {
+        const location = `${path.basename(file)}#${fnName}`;
+        const message = `[${new Date().toISOString()}@${location}] <${this._actionToText(spinnerAction)}> ${text}`;
+        const coloredMessage = color ? chalk.keyword(color)(message) : message;
+        if (!this.isTesting) console.error(coloredMessage);
+      } else if (logLevel === LogLevel.Normal) {
+        const options = color ? { text, status: spinnerAction, color } : { text, status: spinnerAction };
+        !spinners.pick(reference) ? spinners.add(reference, options) : spinners.update(reference, options);
+      }
+    } catch (err) {
+      if (this.isTesting) throw new Error(`Error logging ${reference}: ${err}`);
+      else console.error(`Error logging ${reference}: ${err}`);
     }
   },
 

packages/lib/test/setup.js

@@ -6,7 +6,8 @@ import Contracts from '../src/artifacts/Contracts';
 import { helpers } from '../src/test';
 import { Loggy } from '../src/utils/Logger';
 
-Loggy.silent(true);
+Loggy.silent(false);
+Loggy.testing(true);
 ZWeb3.initialize(web3.currentProvider);
 setArtifactDefaults();
 

packages/lib/test/src/utils/Logger.test.js

@@ -11,25 +11,19 @@ import {
   LogType,
 } from '../../../src/utils/Logger';
 
-describe('Loggger', function() {
-  afterEach('restore logger', function() {
-    Loggy.silent(true);
+describe('Logger', function() {
+  beforeEach(function () {
+    Loggy.testing(false);
+  });
+
+  afterEach('restore logger', function() {    
+    Loggy.testing(true);
+    Loggy.silent(false);
     Loggy.verbose(false);
     Loggy.logs = {};
     sinon.restore();
   });
 
-  describe('default attributes', function() {
-    it('is silent and non-verbose by default', function() {
-      Loggy.isSilent.should.be.true;
-      Loggy.isVerbose.should.be.false;
-    });
-
-    it(`has a 'logs' object initialized`, function() {
-      Loggy.logs.should.be.an('object').that.is.empty;
-    });
-  });
-
   describe('methods', function() {
     describe('#silent', function() {
       it('changes isSilent property value', function() {
@@ -41,7 +35,14 @@ describe('Loggger', function() {
     describe('#verbose', function() {
       it('changes isVerbose property value', function() {
         Loggy.verbose(true);
-        Loggy.isSilent.should.be.true;
+        Loggy.isVerbose.should.be.true;
+      });
+    });
+
+    describe('#testing', function() {
+      it('changes isTesting property value', function() {
+        Loggy.testing(true);
+        Loggy.isTesting.should.be.true;
       });
     });