Reputation: 48490
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
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
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.
Added required error handling.
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.
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