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

@@ -275,7 +275,7 @@ export default class NetworkController {
     const libInstance = await this.project.setImplementation(libClass, libName);
     this.networkFile.addSolidityLib(libName, libInstance);
     Loggy.succeed(
-      `upload-solidity-lob${libName}`,
+      `upload-solidity-lib${libName}`,
       `${libName} library uploaded`,
     );
   }

packages/cli/test/setup.js

@@ -9,7 +9,8 @@ useTestZosPackageFile();
 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

@@ -64,6 +64,12 @@ class BaseProxyAdminProject extends BaseSimpleProject {
       pAddress,
       initCallData,
     } = await this._setUpgradeParams(proxyAddress, contract, contractParams);
+    Loggy.spin(
+      __filename, 
+      'upgradeProxy', 
+      `action-proxy-${pAddress}`, 
+      `Upgrading instance at ${pAddress}`
+    );
     await this.proxyAdmin.upgradeProxy(
       pAddress,
       implementationAddress,

packages/lib/src/project/SimpleProject.ts

@@ -26,10 +26,16 @@ export default class SimpleProject extends BaseSimpleProject {
       pAddress,
       initCallData,
     } = await this._setUpgradeParams(proxyAddress, 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}`,
+      `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,
@@ -117,26 +122,31 @@ 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;
+        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;
       });
     });