Skip to content

Commit

Permalink
fix(@embark/solidity): ensure SolidityProcess receives proper Logger …
Browse files Browse the repository at this point in the history
…instance

Prior to this commit Embark had a bug where, when running `$ embark run` inside
an Embark project that was freshly cloned or reset, it'd get stuck at loading
the Solidity compiler.

However, this only seemed to happen when Embark was used with its dashboard. When
running

```
$ embark run --nodashboard
```

instead, Embark loaded Solidity successfully and moved on with compilation.

This bug pretty much boiled down to `SolidityProcess` not receiving a valid `Logger`
dependency when instantiated via `SolcW`. The reason this was happening
is that we were sending the needed dependencies via `ProcessLauncher.send()`, which
serializes/deserializes its sent data.

```
solidityProcessLauncher.send({action: 'init', options: { logger: this.logger }});
```

And inside `SolidityProcess`

```
process.on('message', (msg) => {
  if (msg.action === "init") {
    solcProcess = new SolcProcess(msg.options);
  }
  ...
}
```

`SolcProcess` passes its `logger` instance down to `longRunningProcessTimer` which
uses methods on `Logger`.

However, since the data had been serialized, all prototype methods on the data is
gone, resulting in an error, which made Embark stop.

**Why was this only a problem when using the dashboard?**

Turns out we've only relied on `Logger.info()` in case the dashboard is used.
  • Loading branch information
0x-r4bbit authored and iurimatias committed Feb 13, 2019
1 parent 193abd4 commit fdd51cf
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 2 deletions.
1 change: 1 addition & 0 deletions packages/embark/src/lib/modules/solidity/solcP.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class SolcProcess extends ProcessWrapper {
let solcProcess;
process.on('message', (msg) => {
if (msg.action === "init") {
msg.options.logger = console;
solcProcess = new SolcProcess(msg.options);
return process.send({result: "initiated"});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/embark/src/lib/modules/solidity/solcW.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class SolcW {
done();
});

this.solcProcess.send({action: "init", options: {logger: self.logger, showSpinner: !self.useDashboard}});
this.solcProcess.send({action: "init", options: {showSpinner: !self.useDashboard}});

if (this.ipc.isServer()) {
this.ipc.on('compile', self.compile.bind(this));
Expand Down
2 changes: 1 addition & 1 deletion packages/embark/src/lib/utils/longRunningProcessTimer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default class LongRunningProcessTimer {
}

// log our measurement and make it red if it has taken too long
if (!this.options.showSpinner && entry && strDuration && this.options && this.options.longRunningThreshold) {
if (!this.options.showSpinner && entry && strDuration !== undefined && this.options && this.options.longRunningThreshold) {
if (entry.duration > this.options.longRunningThreshold) {
strDuration = strDuration.red;
}
Expand Down

0 comments on commit fdd51cf

Please sign in to comment.