fix(@embark/embark-deploy-tracker): Fix reading of empty file (#1872)

There was an error that would display on the second+ run of embark, that was causing by trying to read JSON of an empty file.

The solution was a combination of ensuring the file existed with defaults when enabled, and also ensuring we await the saving of the file.

Included is a bit of a refactor of how the tracking functions handled the “current chain”. Hopefully, this should make things more clear.

Tests have been updated accordingly.

packages/embark-deploy-tracker/src/index.js

@@ -13,7 +13,6 @@ class DeployTracker {
     const trackingFunctions = new TrackingFunctions({config, fs, logger, events, env, trackContracts});
     const deploymentChecks = new DeploymentChecks({trackingFunctions, logger, events, plugins});
 
-    this.embark.events.on("blockchain:started", trackingFunctions.ensureChainTrackerFile.bind(trackingFunctions));
     this.embark.registerActionForEvent("deployment:contract:deployed", trackingFunctions.trackAndSaveContract.bind(trackingFunctions));
     this.embark.registerActionForEvent("deployment:contract:shouldDeploy", deploymentChecks.checkContractConfig.bind(deploymentChecks));
     this.embark.registerActionForEvent("deployment:contract:shouldDeploy", deploymentChecks.checkIfAlreadyDeployed.bind(deploymentChecks));

packages/embark-deploy-tracker/src/test/deploymentChecksSpec.js

@@ -18,6 +18,7 @@ describe('embark.deploymentChecks', function () {
   let contractInChainsFake;
   let chainsFake;
   let trackedContract;
+  let exists;
   let readJSON;
   let writeJSON;
   let _web3;
@@ -50,6 +51,7 @@ describe('embark.deploymentChecks', function () {
         }
       }
     };
+    exists = sinon.stub(fs, 'exists').returns(true);
     readJSON = sinon.stub(fs, 'readJSON').returns(chainsFake);
     writeJSON = sinon.stub(fs, 'writeJSON');
     trackingFunctions = new TrackingFunctions({
@@ -79,6 +81,7 @@ describe('embark.deploymentChecks', function () {
     deploymentChecks._web3 = _web3;
   });
   afterEach(() => {
+    exists.restore();
     readJSON.restore();
     writeJSON.restore();
   });
@@ -191,7 +194,7 @@ describe('embark.deploymentChecks', function () {
       });
     });
     it("should error (and not deploy) if tracked contract address is invalid", async function () {
-      trackingFunctions._web3.eth.getCode = () => { throw new Error(); };
+      trackingFunctions._web3.eth.getCode = () => {throw new Error();};
       return deploymentChecks.checkIfAlreadyDeployed(params, (err, _params) => {
         expect(err).to.not.be(null);
       });

packages/embark-deploy-tracker/src/test/trackingFunctionsSpec.js

@@ -99,6 +99,13 @@ describe('embark.trackingFunctions', function () {
       expect(readJSON.calledOnceWith(chainsPath)).to.be(false);
       expect(chains).to.be(null);
     });
+    it("non-existant chains file path, should not read chains.json and return null", async function () {
+      exists.restore();
+      exists = sinon.stub(fs, 'exists').returns(false);
+      const chains = await trackingFunctions.chains;
+      expect(readJSON.calledOnceWith(chainsPath)).to.be(false);
+      expect(chains).to.be(null);
+    });
     it("trackContracts is false, should not read chains.json and return null", async function () {
       trackingFunctions = new TrackingFunctions({
         config: {
@@ -125,10 +132,17 @@ describe('embark.trackingFunctions', function () {
       expect(currentChain).to.be.equal(contractInChainsFake);
     });
 
-    it("should return emtpy contracts when not enabled", async function () {
+    it("should return null when not enabled", async function () {
       trackingFunctions.enabled = false;
       const currentChain = await trackingFunctions.currentChain;
-      expect(currentChain).to.be.eql({contracts: []});
+      expect(currentChain).to.be(null);
+    });
+
+    it("should return undefined when there is no chains file", async function () {
+      exists.restore();
+      exists = sinon.stub(fs, 'exists').returns(false);
+      const currentChain = await trackingFunctions.currentChain;
+      expect(currentChain).to.be(undefined);
     });
   });
 
@@ -196,7 +210,8 @@ describe('embark.trackingFunctions', function () {
   describe('#ensureChainTrackerFile', function () {
     it("should do nothing when disabled", async function () {
       trackingFunctions.enabled = false;
-
+      exists.restore();
+      exists = sinon.stub(fs, 'exists').returns(true);
       await trackingFunctions.ensureChainTrackerFile();
       expect(exists.called).to.be(false);
     });
@@ -205,7 +220,12 @@ describe('embark.trackingFunctions', function () {
       exists.restore();
       exists = sinon.stub(fs, 'exists').returns(false);
       await trackingFunctions.ensureChainTrackerFile();
-      expect(outputJSON.calledOnceWith(chainsPath, {})).to.be(true);
+      expect(outputJSON.calledOnceWith(chainsPath, {
+        "0x7aec9250dcc5f6bedc3d0d582e0be8b8d159a4d483c47309e122ba5702ec6a16": {
+          contracts: {},
+          name: "development"
+        }
+      })).to.be(true);
     });
   });
 
@@ -254,6 +274,12 @@ describe('embark.trackingFunctions', function () {
       expect(writeJSON.called).to.be(false);
     });
 
+    it("should not save when chains is null", async function () {
+      exists.restore();
+      exists = sinon.stub(fs, 'exists').returns(false);
+      expect(writeJSON.called).to.be(false);
+    });
+
     it("should save to chains.json", async function () {
       await trackingFunctions.save();
       expect(writeJSON.calledOnceWith(chainsPath, chainsFake, {spaces: 2})).to.equal(true);

packages/embark-deploy-tracker/src/trackingFunctions.js

@@ -14,6 +14,9 @@ export default class TrackingFunctions {
     this._web3 = null;
     this._chains = null;
     this._currentChain = null;
+    this._block = null;
+
+    this.ensureChainTrackerFile();
   }
 
   get web3() {
@@ -31,44 +34,56 @@ export default class TrackingFunctions {
       if (this._chains) {
         return this._chains;
       }
-      if (!this.enabled) {
+
+      const exists = await this.fs.exists(this.chainsFilePath);
+      if (!this.enabled || !exists) {
         this._chains = null;
         return this._chains;
       }
+
       this._chains = await this.fs.readJSON(this.chainsFilePath);
       return this._chains;
     })();
   }
 
+  set chains(chains) {
+    this._chains = chains;
+    // chains has changed, therefore currentChain should too
+    // reset the backing variable for currentChain so it can be recalculated on next get
+    this._currentChain = null;
+  }
+
   get currentChain() {
     return (async () => {
       if (this._currentChain) {
         return this._currentChain;
       }
+
       if (!this.enabled) {
-        this._currentChain = {contracts: []};
-        return this._currentChain;
+        return null;
       }
 
-      let block;
+      const chains = (await this.chains) || {};
+      const {hash} = await this.block;
+      this._currentChain = chains[hash];
+      return this._currentChain;
+    })();
+  }
+
+  get block() {
+    return (async () => {
+      if (this._block) {
+        return this._block;
+      }
       const web3 = await this.web3;
       try {
-        block = await web3.eth.getBlock(0, true);
+        this._block = await web3.eth.getBlock(0, true);
       } catch (err) {
         // Retry with block 1 (Block 0 fails with Ganache-cli using the --fork option)
-        block = await web3.eth.getBlock(1, true);
+        this._block = await web3.eth.getBlock(1, true);
+      } finally {
+        return this._block;
       }
-      const {hash} = block;
-      this._currentChain = (await this.chains)[hash];
-      if (this._currentChain === undefined) {
-        const empty = {contracts: {}};
-        this._chains[hash] = empty;
-        this._currentChain = empty;
-      }
-
-      this._currentChain.name = this.env;
-
-      return this._currentChain;
     })();
   }
 
@@ -86,7 +101,7 @@ export default class TrackingFunctions {
     const {contract} = params;
     if (!this.enabled) return cb();
     await this.trackContract(contract);
-    this.save();
+    await this.save();
     cb();
   }
 
@@ -95,7 +110,13 @@ export default class TrackingFunctions {
     const exists = await this.fs.exists(this.chainsFilePath);
     if (!exists) {
       this.logger.info(this.chainsFilePath + ' ' + __('file not found, creating it...'));
-      return this.fs.outputJSON(this.chainsFilePath, {});
+      const {hash} = await this.block;
+      return this.fs.outputJSON(this.chainsFilePath, {
+        [hash]: {
+          contracts: {},
+          name: this.env
+        }
+      });
     }
   }
 
@@ -104,13 +125,17 @@ export default class TrackingFunctions {
     if (!this.enabled || !currentChain) return false;
     const toTrack = {name: contract.className, address: contract.deployedAddress};
     if (contract.track === false) toTrack.track = false;
-    currentChain.contracts[contract.hash] = toTrack;
+    const {hash} = await this.block;
+    const chains = await this.chains;
+    chains[hash].contracts[contract.hash] = toTrack;
+    this.chains = chains;
   }
 
   async save() {
-    if (!this.enabled) {
+    const chains = await this.chains;
+    if (!this.enabled || !chains) {
       return;
     }
-    return this.fs.writeJSON(this.chainsFilePath, await this.chains, {spaces: 2});
+    return this.fs.writeJSON(this.chainsFilePath, chains, {spaces: 2});
   }
 }