diff --git a/.gitignore b/.gitignore index b88ef2d..d8720ac 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ # Logs logs *.log +.DS_Store # Runtime data pids @@ -31,4 +32,4 @@ node_modules test/temp/*.* lib/scripts/serverScript.js -lib/scripts/standaloneScript.js \ No newline at end of file +lib/scripts/standaloneScript.js diff --git a/lib/conversion.js b/lib/conversion.js index 3a49037..2c86aa2 100644 --- a/lib/conversion.js +++ b/lib/conversion.js @@ -65,9 +65,45 @@ function convert(conversionOptions, cb) { delete opt.html; if (options.strategy === "phantom-server") - return require("./serverStrategy.js")(options, opt, id, cb); + return require("./serverStrategy.js")(options, opt, id, checkIfShouldRemoveTempFile); if (options.strategy === "dedicated-process") - return require("./dedicatedProcessStrategy.js")(options, opt, id, cb); + return require("./dedicatedProcessStrategy.js")(options, opt, id, checkIfShouldRemoveTempFile); + + // verify if we should remove the pdf temp file, + // only if the user has not done any intention to consume the file stream + function checkIfShouldRemoveTempFile(err, resp) { + var shouldRemoveTempFile = false; + + if (err) { + return cb(err); + } + + if (resp.stream == null) { + return cb(null, resp); + } + + // executing the callback + cb(null, resp); + + // on the next tick of the event loop verify if file stream is consumed.. + process.nextTick(function() { + if (resp.stream._readableState.flowing === false && + resp.stream._readableState.reading === false && + resp.stream._readableState.calledRead === false && + resp.stream._readableState.needReadable === false) { + // in node 0.10.x these properties tell if a stream is not consumed either on flowing or paused mode + shouldRemoveTempFile = true; + } else if (resp.stream._readableState.flowing === null && + resp.stream._readableState.needReadable === false) { + // in node 0.12.x and 4.x.x, 5.x.x, 6.x.x, 7.x.x ... these properties tell if a stream is not consumed either on flowing or paused mode + shouldRemoveTempFile = true; + } + + if (shouldRemoveTempFile) { + fs.unlink(resp.stream.path, function() { /* ignore any error when deleting the file */ }); + } + }); + } cb(new Error("Unsupported strategy " + options.strategy)); }); @@ -94,4 +130,3 @@ module.exports = function (opt) { return convert; }; - diff --git a/lib/dedicatedProcessStrategy.js b/lib/dedicatedProcessStrategy.js index 403b2b4..59457f7 100644 --- a/lib/dedicatedProcessStrategy.js +++ b/lib/dedicatedProcessStrategy.js @@ -1,6 +1,7 @@ var path = require("path"), childProcess = require('child_process'), - fs = require("fs"); + fs = require("fs"), + removeTempFileWhenConsumed = require('./removeTempFileWhenConsumed'); module.exports = function(options, requestOptions, id, cb) { @@ -58,8 +59,12 @@ module.exports = function(options, requestOptions, id, cb) { m.timestamp = new Date(m.timestamp) }) + var outputStream = fs.createReadStream(requestOptions.output); + + removeTempFileWhenConsumed(requestOptions.output, outputStream); + cb(null, { - stream: fs.createReadStream(requestOptions.output), + stream: outputStream, numberOfPages: response.numberOfPages, logs: response.logs }); diff --git a/lib/removeTempFileWhenConsumed.js b/lib/removeTempFileWhenConsumed.js new file mode 100644 index 0000000..8b06804 --- /dev/null +++ b/lib/removeTempFileWhenConsumed.js @@ -0,0 +1,34 @@ +var fs = require('fs'), + firstEvent = require('ee-first'); + +module.exports = function removeTempFileWhenConsumed(filepath, fileStream) { + var thunk = firstEvent([ + [fileStream, 'close', 'end', 'error'] + ], function(err, stream, eventName, args) { + var shouldPropagateStreamError = false; + + if (err || eventName === 'error') { + // if there is only one listener for the 'error' event (our handler) + // we should propagate the error, + // if there is more than one listener it means that the user is listening for the event 'error' + // in the stream, so it's user responsibility to handle the error correctly + if (fileStream.listeners('error').length === 1) { + shouldPropagateStreamError = true; + } + } + + // clean up event listeners on stream after the callback has been executed + thunk.cancel(); + + // clean up temp file + deleteFile(filepath); + + if (shouldPropagateStreamError) { + fileStream.emit('error', err); + } + }); +}; + +function deleteFile(filepath) { + fs.unlink(filepath, function() { /* ignore any error when deleting the file */ }); +} diff --git a/lib/serverStrategy.js b/lib/serverStrategy.js index 514ebbd..cd94fe6 100644 --- a/lib/serverStrategy.js +++ b/lib/serverStrategy.js @@ -1,6 +1,7 @@ var Phantom = require("phantom-workers"), fs = require("fs"), - _ = require("lodash"); + _ = require("lodash"), + removeTempFileWhenConsumed = require('./removeTempFileWhenConsumed'); var phantoms = {}; @@ -61,8 +62,12 @@ module.exports = function(options, requestOptions, id, cb) { m.timestamp = new Date(m.timestamp) }) + var outputStream = fs.createReadStream(requestOptions.output); + + removeTempFileWhenConsumed(requestOptions.output, outputStream); + cb(null, { - stream: fs.createReadStream(requestOptions.output), + stream: outputStream, numberOfPages: res.numberOfPages, logs: res.logs }); diff --git a/package.json b/package.json index c5d7344..e74ac4a 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "url": "git@github.com:pofider/phantom-html-to-pdf.git" }, "dependencies": { + "ee-first": "1.1.1", "in-publish": "2.0.0", "lodash": "4.13.1", "phantom-workers": "0.4.0", diff --git a/test/test.js b/test/test.js index c566114..5ebf697 100644 --- a/test/test.js +++ b/test/test.js @@ -77,6 +77,65 @@ describe("phantom html to pdf", function () { }); }); + it("should remove temp pdf file when stream is consumed in flowing mode", function (done) { + conversion("foo", function (err, res) { + if (err) + return done(err); + + res.numberOfPages.should.be.eql(1); + + // switching the stream to flowing mode, to consume its content + res.stream.on('data', function() {}); + + setTimeout(function() { + // temp pdf file should be removed after being consumed + should(fs.existsSync(res.stream.path)).be.eql(false); + + done(); + }, 300); + }); + }); + + it("should remove temp pdf file when stream is consumed in paused mode", function (done) { + conversion("foo", function (err, res) { + if (err) + return done(err); + + res.numberOfPages.should.be.eql(1); + + // consuming the stream from paused mode + res.stream.on('readable', function() { + var chunk; + + while (null !== (chunk = res.stream.read())) { + } + }); + + setTimeout(function() { + // temp pdf file should be removed after being consumed + should(fs.existsSync(res.stream.path)).be.eql(false); + + done(); + }, 300); + }); + }); + + it("should remove temp pdf file when not consumed", function (done) { + conversion("foo", function (err, res) { + if (err) + return done(err); + + res.numberOfPages.should.be.eql(1); + + setTimeout(function() { + // temp pdf file should be removed after being consumed + should(fs.existsSync(res.stream.path)).be.eql(false); + + done(); + }, 300); + }); + }); + it("should work with multiple phantom paths", function (done) { conversion({ html: "foo", phantomPath: phantomjs.path}, function (err, res) { if (err) @@ -261,4 +320,4 @@ describe("phantom html to pdf", function () { } } }; -}); \ No newline at end of file +});