randombits
randombits

Reputation: 48490

Promisfy writing file to filesystem

I am interested in understanding how to Promisfy this block of code:

const http = require('http');
const fs = require('fs');

const download = function(url, dest, cb) {
  let file = fs.createWriteStream(dest);
  const request = http.get(url, function(response) {
    response.pipe(file);
    file.on('finish', function() {
      file.close(cb);  // close() is async, call cb after close completes.
    });
  }).on('error', function(err) { // Handle errors
    fs.unlink(dest); // Delete the file async. (But we don't check the result)
    if (cb) cb(err.message);
  });
};

My first take on this was something to the extent of:

const http = require('http');
const fs = require('fs');

const download = async (url, dest, cb) => {
  let file = fs.createWriteStream(dest);
  const request = http.get(url, function(response) {
    response.pipe(file);
    file.on('finish', function() {
      const closed = await file.close(cb);  // close() is async, await here?
      if (closed) {
          // handle cleanup and retval
      }
    });
  }).on('error', function(err) { // Handle errors
    const deleted = await fs.unlink(dest); // Delete the file async. 
    if (!deleted) { ... }
  });
};

The implementation above is clearly wrong. What is the right away to approach this to remove callbacks and just use async/await?

Upvotes: 1

Views: 638

Answers (2)

Patrick Roberts
Patrick Roberts

Reputation: 51946

Here's how I'd re-write your node-style callback API as an asynchronous function:

const http = require('http');
const fs = require('fs');

async function download (url, dest) {
  const response = await new Promise((resolve, reject) => {
    http.get(url, resolve).once('error', reject);
  });

  if (response.status < 200 || response.status >= 300) {
    throw new Error(`${responses.status} ${http.STATUS_CODES[response.status]}`);
  }

  const file = await fs.promises.open(dest, 'w');

  try {
    for await (const data of response) {
      await file.write(data);
    }
  } catch (error) {
    await file.close();
    await fs.promises.unlink(dest);
    throw error;
  }

  await file.close();
}

Note that this approach uses the FileHandle class in the fs.promises namespace, as well as the Symbol.asyncIterator interface defined on the Readable stream class, which allows you to consume the data events of the response with a for await...of loop and propagate error handling from the error event of the response to the catch block by implicitly rejecting the promise returned by the underlying asynchronous iterator.

Upvotes: 3

jfriend00
jfriend00

Reputation: 707916

Here's a way to manually wrap the pipe operation in a promise. Unfortunately, most of this is just error handling to cover all the possible places an error can occur:

const http = require('http');
const fs = require('fs');

const download = function(url, dest) {
  return new Promise((resolve, reject) => {
      const file = fs.createWriteStream(dest);

      // centralize error cleanup function
      function cleanup(err) {
          reject(err);
          // cleanup partial results when aborting with an error
          file.on('close', () => {
            fs.unlink(dest);
          });
          file.end();
      }

      file.on('error', cleanup).on('finish', resolve);

      const request = http.get(url, function(response) {
        if (response.status < 200 || response.status >= 300) {
            cleanup(new Error(`Unexpected Request Status Code: ${response.status}`);
            return;
        }
        response.pipe(file);
        response.on('error', cleanup);

      }).on('error', cleanup);
  });
};

download(someURL, someDest).then(() => {
  console.log("operation complete");
}).catch(err => {
  console.log(err);
});

This does not wait for files to be closed or removed in the error conditions before rejecting (figuring that there's typically nothing constructive to do if those cleanup operations have errors anyway). If that is desired, it could be added easily by just calling reject(err) from the asynchronous callbacks for those cleanup operations or by using the fs.promises version of those functions and awaiting them.


A few things to note. This is mostly error handling because there are three possible places you can have errors and some cleanup work needed for some errors.

  1. Added required error handling.

  2. In the OP's original code, they called file.close(), but file is a stream and there is no .close() method on a writeStream. You call .end() to close the write stream.

  3. You also probably need to check for an appropriate response.status because http.get() still returns a response object and stream even if the status is something like 4xx or 5xx.

Upvotes: 3

Related Questions