Fix how native solc is checked after download

[fvictorio]

Use util.promisify instead of implementing our own handling of the execFile result. This fixes #3195 because of some very fun reasons.

tl;dr

The previous implementation wasn't handling the case were the ChildProcess instance emitted an "error" event. You would expect that this would hang the process forever, but you would be wrong (I know I was).

The reason it was erroring is that, in an alpine-based docker image, the solc binary couldn't even be executed. This caused an error instead of emitting an "exit" event with a non-zero code.

Explanation

The problem here is a combination of two weird things I had no idea about.

First, when you download a solc native binary in an alpine-based docker container, the file cannot be executed. The fun part is that execFile errors with a ENOENT error, and running it in the shell fails with a "file not found" error. This was already misleading enough. Here's a good explanation of why that happens.

Second, and most important: in that scenario the ChildProcess instance returned by execFile would emit an error event, and we weren't handling that. In my mental model of how node.js works, this should've caused the Hardhat process to hang forever, but my mental model was wrong. Unresolved promises don't prevent the node process from finishing if there's no pending IO or timeouts or whatever, as explained here.

In other words: hh compile was downloading a binary that didn't work and finishing with a successful exit code. After that, successive executions would try to use that compiler and fail.

The lesson I take from this is that we should be super careful when we convert something like an event emitter into a promise.

[linear]

HH-1188 Hardhat 2.11 fails in docker environments [NomicFoundation/hardhat#3195]

Reproduction: https://github.com/DefiCake/hardhat-211-docker-bug-report

I am finding troubles when running hardhat inside CI pipelines. I am not sure where this was introduced, suspecting the latest 2.11 where compile time optimizations happened, but not really sure. Got a personal project at v.2.9.9 where it does work well.

It boils down to it failing when running in dockerized envs with either one of these two errors:

Either it will exit early during solc download and not run the test:

test_1  | yarn run v1.22.19
test_1  | $ hardhat test
test_1  | Downloading compiler 0.8.17
test_1  | Done in 1.19s.
hardhat-211-compiler-bug_test_1 exited with code 0

Or it will fail with an EPIPE error:

test_1  | yarn run v1.22.19
test_1  | $ hardhat test
test_1  | Error: write EPIPE
test_1  |     at afterWriteDispatched (internal/stream_base_commons.js:156:25)
test_1  |     at writeGeneric (internal/stream_base_commons.js:147:3)
test_1  |     at Socket._writeGeneric (net.js:798:11)
test_1  |     at Socket._write (net.js:810:8)
test_1  |     at writeOrBuffer (internal/streams/writable.js:358:12)
test_1  |     at Socket.Writable.write (internal/streams/writable.js:303:10)
test_1  |     at output (/app/node_modules/hardhat/src/internal/solidity/compiler/index.ts:82:24)
test_1  |     at new Promise (<anonymous>)
test_1  |     at NativeCompiler.compile (/app/node_modules/hardhat/src/internal/solidity/compiler/index.ts:66:34)
test_1  |     at SimpleTaskDefinition.action (/app/node_modules/hardhat/src/builtin-tasks/compile.ts:639:37)
test_1  | error Command failed with exit code 1.
test_1  | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
hardhat-211-compiler-bug_test_1 exited with code 1

Docker info:
Docker version 20.10.18, build b40c2f6
docker-compose version 1.29.2, build 5becea4c

---

Github issue/pull request

This issue was automatically created, do not edit its description.

GH-ID: #3195

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
hardhat	✅ Ready (Inspect)	Visit Preview	Oct 19, 2022 at 0:08AM (UTC)
hardhat-storybook	✅ Ready (Inspect)	Visit Preview	Oct 19, 2022 at 0:08AM (UTC)

[changeset-bot]

🦋 Changeset detected

Latest commit: 145b12c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
hardhat	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[alifarooq0]

Thanks for fixing this @fvictorio!

[fvictorio]

@alifarooq0 my pleasure! This will need a review from @alcuadrado, who is on vacation this week. Hopefully we can release it mid next week.

[fvictorio]

I wonder if we should prevent this from ever happening, at least in the CLI. There doesn't seem to be a very good way of doing it though.

One option:

let finished = false;

process.on("exit", () => {
  if (!finished) {
    console.error("shouldn't happen");
    process.exitCode = 1;
  }
});

main()
  .then(() => {
    finished = true;
    process.exit(process.exitCode)
  })
  .catch((error) => {
    finished = true;
    console.error(error);
    process.exit(1);
  });

[alifarooq0]

@fvictorio to better understand the calling code, is the issue that we need to bubble up the error from _checkNativeSolc if there is one?

[fvictorio]

Not really bubble up, it was just about handling it. We were using execFile to test if the binary was working correctly (running solc --version basically), but that method returns a ChildProcess, which is an event emitter that can emit events like exit and error. We were only handling the exit one.

[alcuadrado]

Approving it.

Note that this works because child_process.execFile has a custom util.promisify behavior https://nodejs.org/docs/latest-v14.x/api/child_process.html#child_process_child_process_execfile_file_args_options_callback