Replace synchronous file operations with async versions

[catzhang]

Fixes #27

[catzhang]

I wasn't too sure about best practices for throwing errors, logging to the console, and converting the readFile call, so please let me know what to change!

[reberhardt7]

Thanks for looking into this! Sorry it took me so long to review. Looks great, except I'd like to convert callbacks to promises and use async/await syntax

[reberhardt7]

I'd rename errPathExists to something like accessError, since the "PathExists" part could imply that the path does exist

[reberhardt7]

The way that Javascript does concurrency can be super confusing at first. We want to do this sequentially:

• Check if the directory exists
• Create it if it doesn't
• Write the file

It might look like this code does that, but actually it does this:

• Check if the directory exists
  • Whenever the check completes, create it if it doesn't
• Immediately proceed to write the file

If we could only use callbacks, we would need to put all code following fs.access inside the callback function. This gets ugly pretty quickly. Instead, this codebase uses promises with async/await syntax. The fs.promises module contains a bunch of versions of fs calls that return promises instead of executing callbacks.

We should be able to do something like this:

try {
    await fs.promises.mkdir(this.dataHostPath);
} catch (err) {
    // Ignore the error if the directory already exists. Otherwise, re-throw it
    if (err && err.code != 'EEXIST') {
        throw err;
    }
}
await fs.promises.writeFile(this.codeHostPath, code);
...

The fs.promises calls return a promise, which is an object that indicates whether the operation has finished (and any returned values or errors from the underlying callback). Then, await says to wait until the promise resolves before executing the subsequent code. (It's actually quite a bit more involved under the hood, but this is the basic idea.)

You'll also need to convert saveCodeFiles to be an async function by adding the async keyword:

private saveCodeFiles = async (code: string, includeFileData: Buffer | null): void => ...

Now that saveCodeFiles is an async function, it returns a promise right away (before it finishes executing) instead of executing synchronously. That means you'll need to add await everywhere it's called.

It looks like it gets called from the Container constructor. Unfortunately constructors cannot be async functions (Javascript requires that objects are constructed synchronously), so you won't be able to use await there. However, since async just makes a function return an ordinary promise, you can do something like this:

this.saveCodeFiles(code, includeFileData)
    .then(enableDebugging ? this.initializeDebugSocket : Promise.resolve))
    .then(() => this.startContainer(compiler, debugAwareCflags, argsStr, rows, cols, enableDebugging));

Or, since I think we're starting to get several async-y things that we need to do in the constructor, we could define an async function inside of the constructor and invoke that:

// towards end of constructor
(async (): void => {
    await this.saveCodeFiles(code, includeFileData);
    if (enableDebugging) {
        await this.initializeDebugSocket();
    }
    await this.startContainer(.....);
})().then(); // the .then() is just to suppress a warning about ignoring the promise returned by the async function

[reberhardt7]

Same comments here about using async/await with promises instead of callbacks