From 61c7ecd506c4f6c86c98060b4574d7d3a75cce9d Mon Sep 17 00:00:00 2001 From: Amoki Date: Tue, 26 Aug 2014 16:13:55 +0200 Subject: [PATCH 1/7] replace domain with fork --- lib/helpers/child-process.js | 19 ++++++ lib/helpers/hydrater.js | 95 ++++++++++++++++---------- lib/index.js | 1 + test/errors.js | 26 ++----- test/helpers/hydrater.js | 35 +++------- test/hook.js | 2 +- test/hydraters/buggy-hydrater.js | 3 + test/hydraters/dummy-hydrater.js | 4 ++ test/hydraters/errored-hydrater.js | 6 ++ test/hydraters/too-long-hydrater.js | 6 ++ test/hydraters/update-date-hydrater.js | 4 ++ test/index.js | 9 +-- 12 files changed, 119 insertions(+), 91 deletions(-) create mode 100644 lib/helpers/child-process.js create mode 100644 test/hydraters/buggy-hydrater.js create mode 100644 test/hydraters/dummy-hydrater.js create mode 100644 test/hydraters/errored-hydrater.js create mode 100644 test/hydraters/too-long-hydrater.js create mode 100644 test/hydraters/update-date-hydrater.js diff --git a/lib/helpers/child-process.js b/lib/helpers/child-process.js new file mode 100644 index 0000000..a3bf89c --- /dev/null +++ b/lib/helpers/child-process.js @@ -0,0 +1,19 @@ +"use strict"; +var async = require("async"); + +process.on('message', function(task) { + var hydrate = require(task.functionPath); + async.waterfall([ + function startHydration(cb) { + cb.urlCallback = task.options.urlCallback; + cb.apiUrl = task.options.apiUrl; + hydrate(task.path, task.document, task.changes, cb); + } + ], + function(err, changes) { + process.send({ + err: err, + changes: changes + }); + }); +}); diff --git a/lib/helpers/hydrater.js b/lib/helpers/hydrater.js index e259690..8cf9460 100644 --- a/lib/helpers/hydrater.js +++ b/lib/helpers/hydrater.js @@ -8,7 +8,7 @@ */ var async = require('async'); -var nodeDomain = require('domain'); +var shellFork = require('child_process').fork; var request = require('supertest'); var crypto = require('crypto'); var restify = require('restify'); @@ -74,8 +74,9 @@ module.exports = function(hydraterFunction, logger, errLogger) { } }, function performHydration(cb) { - var domain = nodeDomain.create(); - + var child = shellFork(__dirname + '/child-process.js', {silent: true}); + var stderr = ""; + var stdout = ""; var timeout; /** * Function to call, either on domain error, on hydration error or successful hydration. @@ -84,46 +85,65 @@ module.exports = function(hydraterFunction, logger, errLogger) { var cleaner = function(err, changes) { if(!cleaner.called) { cleaner.called = true; - domain.exit(); - domain.dispose(); + child.kill('SIGKILL'); cb(err, changes); } + clearTimeout(timeout); }; cleaner.called = false; - domain.on('error', cleaner); - - // Run in a domain to prevent memory leak on crash - domain.run(function() { - async.waterfall([ - function callHydrationFunction(cb) { - // Give user access to the final URL callback and the API url (which can be staging, prod or anything) - // In case he wants to bypass us and send the changes himself - cb.urlCallback = task.callback; - cb.apiUrl = ''; - if(task.callback) { - var parsed = url.parse(task.callback); - cb.apiUrl = parsed.protocol + "//" + parsed.host; - } - - // Call the real function for hydration. - hydraterFunction((task.file_path) ? path : null, task.document, lib.defaultChanges(), cb); - } - ], function cleanHydration(err, changes) { - // If the function replied with an "HydrationError", we'll wrap this in a nicely formatted document - // and stop the error from bubbling up. - if(err instanceof HydrationError) { - changes = {}; - changes.hydration_errored = true; - changes.hydration_error = err.message; - err = null; - } + child.on('error', function(errCode) { + cleaner(new HydrationError("Wild error appeared during spwaning child. err code:" + errCode)); + }); + + child.stderr.on('readable', function() { + var chunk; + while (null !== (chunk = child.stderr.read())) { + stderr += chunk; + } + }); + + child.stdout.on('readable', function() { + var chunk; + while (null !== (chunk = child.stdout.read())) { + stdout += chunk; + } + }); + + child.on('exit', function(errCode) { + if(errCode !== 0) { + cleaner(new HydrationError(["Child exiting with err code: " + errCode, stdout, stderr])); + } + }); + + // Build objects to send to child + var options = {}; + options.urlCallback = task.callback; + options.apiUrl = ''; + if(task.callback) { + var parsed = url.parse(task.callback); + options.apiUrl = parsed.protocol + "//" + parsed.host; + } + + child.send({ + functionPath: hydraterFunction, + path: (task.file_path) ? path : null, + document: task.document, + changes: lib.defaultChanges(), + options: options, + }); + + child.on('message', function(res) { + // If the function replied with an "HydrationError", we'll wrap this in a nicely formatted document + // and stop the error from bubbling up. + if(res.err && res.err._hydrationError === true) { + res.changes = {}; + res.changes.hydration_errored = true; + res.changes.hydration_error = res.err.message; + res.err = null; + } + cleaner(res.err, res.changes); - // Wait for nexttick, to end this function and be able to properly GC it on domain.dispose(). - process.nextTick(function() { - cleaner(err, changes); - }); - }); }); timeout = setTimeout(function() { @@ -183,6 +203,7 @@ module.exports = function(hydraterFunction, logger, errLogger) { } var apiUrl = url.parse(task.callback, false, true); + request(apiUrl.protocol + "//" + apiUrl.host) .patch(apiUrl.path) .send(changes) diff --git a/lib/index.js b/lib/index.js index 6450397..0aee920 100644 --- a/lib/index.js +++ b/lib/index.js @@ -62,5 +62,6 @@ module.exports.defaultChanges = function() { module.exports.HydrationError = function(message) { this.name = 'HydrationError'; this.message = (message || "").toString(); + this._hydrationError = true; }; util.inherits(module.exports.HydrationError, Error); diff --git a/test/errors.js b/test/errors.js index 47d8fdd..82163a5 100644 --- a/test/errors.js +++ b/test/errors.js @@ -6,16 +6,9 @@ var restify = require('restify'); var anyfetchFileHydrater = require('../lib/'); -var buggyHydrater = function() { - // Fake async stuff - process.nextTick(function() { - throw new Error("I'm buggy"); - }); -}; - describe('Errors', function() { var config = { - hydrater_function: buggyHydrater, + hydrater_function: __dirname + '/hydraters/buggy-hydrater.js', logger: function() {// Will be pinged with error. We don't care. }, errLogger: function() {// Will be pinged with error. We don't care. @@ -65,13 +58,13 @@ describe('Errors', function() { .end(done); }); - it('should be handled gracefully if file does not exists', function(done) { + it.only('should be handled gracefully if file does not exists', function(done) { this.timeout(10000); request(hydrationServer).post('/hydrate') .send({ - file_path: 'http://anyfetch.com/NOPE?some_query', - callback: 'http://anyfetch.com/result?some_query', + file_path: 'http://oseftarace.com/NOPE?some_query', + callback: 'http://oseftarace.com/result?some_query', document: { metadata: { "foo": "bar" @@ -88,15 +81,8 @@ describe('Errors', function() { }); describe('hydrationErrors', function() { - var erroredHydrater = function(path, document, changes, cb) { - // Fake async stuff - process.nextTick(function() { - cb(new anyfetchFileHydrater.HydrationError("hydrater errored")); - }); - }; - var config = { - hydrater_function: erroredHydrater, + hydrater_function: __dirname + '/hydraters/errored-hydrater.js', logger: function() {// Will be pinged with error. We don't care. } }; @@ -126,7 +112,7 @@ describe('hydrationErrors', function() { request(hydrationErrorServer).post('/hydrate') .send({ - file_path: 'http://anyfetch.com/', + file_path: 'http://osef.com/', callback: 'http://127.0.0.1:4242/result', document: { metadata: { diff --git a/test/helpers/hydrater.js b/test/helpers/hydrater.js index f7d981a..c95ff91 100644 --- a/test/helpers/hydrater.js +++ b/test/helpers/hydrater.js @@ -1,20 +1,13 @@ 'use strict'; require('should'); - +var path = require("path"); describe("Hydrated document", function() { it("should have only updated changes", function(done) { - - var dummyHydrater = function(path, document, changes, cb) { - changes.metadata.hydrated = true; - - cb(null, changes); - }; - var config = { - hydrater_function: dummyHydrater, + hydrater_function: path.resolve(__dirname, '../hydraters/dummy-hydrater.js'), logger: function(str, err) { if(err) { throw err; @@ -24,7 +17,7 @@ describe("Hydrated document", function() { var hydrate = require('../../lib/helpers/hydrater')(config.hydrater_function, config.logger); var task = { - callback: "http://wedontcare.com", + callback: "http://osef.com", filepath: "/tmp/anyfetch-hydrater.test", document: { id: "azerty" @@ -38,14 +31,8 @@ describe("Hydrated document", function() { }); it("should keep Dates", function(done) { - var dummyHydrater = function(path, document, changes, cb) { - changes.creation_date = new Date(); - - cb(null, changes); - }; - var config = { - hydrater_function: dummyHydrater, + hydrater_function: path.resolve(__dirname, '../hydraters/update-date-hydrater.js'), logger: function(str, err) { if(err) { throw err; @@ -54,7 +41,7 @@ describe("Hydrated document", function() { }; var hydrate = require('../../lib/helpers/hydrater')(config.hydrater_function, config.logger); var task = { - callback: "http://wedontcare.com", + callback: "http://osef.com", filepath: "/tmp/anyfetch-hydrater.test", document: { id: "azerty" @@ -70,17 +57,15 @@ describe("Hydrated document", function() { describe('Timeout', function() { +var shellFork = require('child_process').fork; +var HydrationError = require('../../lib/index.js').HydrationError; + + it('should send an error', function(done) { process.env.TIMEOUT = 20; - var tooLongHydrater = function(path, document, changes, cb) { - setTimeout(function() { - cb(null, changes); - }, 1000); - }; - var config = { - hydrater_function: tooLongHydrater, + hydrater_function: path.resolve(__dirname, '../hydraters/too-long-hydrater.js'), logger: function(str, err) { if(err) { throw err; diff --git a/test/hook.js b/test/hook.js index 6a1a7fe..173fc78 100644 --- a/test/hook.js +++ b/test/hook.js @@ -9,7 +9,7 @@ var fs = require('fs'); var anyfetchFileHydrater = require('../lib/'); -describe('/hydrate webhooks', function() { +describe.skip('/hydrate webhooks', function() { // Patch AnyFetch URL // Avoid uselessly pinging anyfetch.com with invalid requests process.env.ANYFETCH_API_URL = 'http://localhost'; diff --git a/test/hydraters/buggy-hydrater.js b/test/hydraters/buggy-hydrater.js new file mode 100644 index 0000000..9b0e381 --- /dev/null +++ b/test/hydraters/buggy-hydrater.js @@ -0,0 +1,3 @@ +module.exports = function buggyHydrater(path, document, changes, cb) { + throw new Error("I'm buggy"); +}; diff --git a/test/hydraters/dummy-hydrater.js b/test/hydraters/dummy-hydrater.js new file mode 100644 index 0000000..86d1fee --- /dev/null +++ b/test/hydraters/dummy-hydrater.js @@ -0,0 +1,4 @@ +module.exports = function dummyHydrater(path, document, changes, cb) { + changes.metadata.hydrated = true; + cb(null, changes); +}; diff --git a/test/hydraters/errored-hydrater.js b/test/hydraters/errored-hydrater.js new file mode 100644 index 0000000..312651b --- /dev/null +++ b/test/hydraters/errored-hydrater.js @@ -0,0 +1,6 @@ +var anyfetchFileHydrater = require('../../lib/'); + + +module.exports = function erroredHydrater(path, document, changes, cb) { + cb(new anyfetchFileHydrater.HydrationError("hydrater errored")) +}; diff --git a/test/hydraters/too-long-hydrater.js b/test/hydraters/too-long-hydrater.js new file mode 100644 index 0000000..a4b1849 --- /dev/null +++ b/test/hydraters/too-long-hydrater.js @@ -0,0 +1,6 @@ +module.exports = function tooLongHydrater(path, document, changes, cb) { + setTimeout(function() { + changes.changed = true; + cb(null, changes); + }, 1000) +}; diff --git a/test/hydraters/update-date-hydrater.js b/test/hydraters/update-date-hydrater.js new file mode 100644 index 0000000..4ebda1b --- /dev/null +++ b/test/hydraters/update-date-hydrater.js @@ -0,0 +1,4 @@ +module.exports = function updateDateHydrater(path, document, changes, cb) { + changes.creation_date = new Date(); + cb(null, changes); +}; diff --git a/test/index.js b/test/index.js index fdb51c9..765bc87 100644 --- a/test/index.js +++ b/test/index.js @@ -5,15 +5,8 @@ var request = require('supertest'); var async = require('async'); var anyfetchFileHydrater = require('../lib/'); - -var dummyHydrater = function(path, document, changes, cb) { - changes.metadata.hydrated = true; - - cb(null, changes); -}; - var config = { - hydrater_function: dummyHydrater, + hydrater_function: __dirname + '/hydraters/dummy-hydrater.js', logger: function(str, err) { if(err) { throw err; From d17cba51051447f1c4dd82bf6eaa75c36b5922d2 Mon Sep 17 00:00:00 2001 From: Amoki Date: Wed, 27 Aug 2014 14:56:54 +0200 Subject: [PATCH 2/7] first presentable code --- lib/helpers/hydrater.js | 40 +++--- test/errors.js | 202 +++++++++++++++-------------- test/helpers/fake-api.js | 17 +++ test/helpers/hydrater.js | 142 ++++++++++---------- test/hook.js | 100 ++++++-------- test/hydraters/skipper-hydrater.js | 3 + test/hydraters/useful-hydrater.js | 7 + test/index.js | 25 +++- 8 files changed, 282 insertions(+), 254 deletions(-) create mode 100644 test/helpers/fake-api.js create mode 100644 test/hydraters/skipper-hydrater.js create mode 100644 test/hydraters/useful-hydrater.js diff --git a/lib/helpers/hydrater.js b/lib/helpers/hydrater.js index 8cf9460..76f9295 100644 --- a/lib/helpers/hydrater.js +++ b/lib/helpers/hydrater.js @@ -44,35 +44,31 @@ module.exports = function(hydraterFunction, logger, errLogger) { /** * Download the file from task.file_path, store it in a temporary file if there is file_path */ - function initHydration(cb) { + function downloadFile(cb) { if(task.file_path) { - // Download the file - var stream = fs.createWriteStream(path); - - // Store error if statusCode !== 200 - var err; - stream.on("finish", function() { - cb(err); - }); - var apiUrl = url.parse(task.file_path, false, true); - var req = request(apiUrl.protocol + "//" + apiUrl.host) - .get(apiUrl.path); - - req.end().req.once('response', function(res) { - if(res.statusCode !== 200) { - err = new restify.BadGatewayError('Error when downloading file ' + task.file_path + ': ' + res.statusCode); - stream.end(); - this.abort(); - } - }); - - req.pipe(stream); + request(apiUrl.protocol + "//" + apiUrl.host) + .get(apiUrl.path) + .expect(200) + .end(function(err, res) { + if(err) { + err = new restify.BadGatewayError('Error when downloading file ' + task.file_path + ': ' + err); + } + cb(err, res.text); + }); } else { cb(null); } }, + function saveFile(res, cb) { + if(res) { + fs.writeFile(path, res, cb); + } + else{ + cb(); + } + }, function performHydration(cb) { var child = shellFork(__dirname + '/child-process.js', {silent: true}); var stderr = ""; diff --git a/test/errors.js b/test/errors.js index 82163a5..3a11330 100644 --- a/test/errors.js +++ b/test/errors.js @@ -3,7 +3,7 @@ require('should'); var request = require('supertest'); var restify = require('restify'); - +var fs = require("fs"); var anyfetchFileHydrater = require('../lib/'); describe('Errors', function() { @@ -16,115 +16,123 @@ describe('Errors', function() { }; var hydrationServer = anyfetchFileHydrater.createServer(config); - it('should be handled gracefully while hydrating', function(done) { - this.timeout(10000); - request(hydrationServer).post('/hydrate') - .send({ - file_path: 'http://anyfetch.com', - callback: 'http://anyfetch.com/result', - document: { - metadata: { - "foo": "bar" - } - } - }) - .expect(204) - .end(function() {}); - hydrationServer.queue.drain = function(err) { - hydrationServer.queue.drain = null; - done(err); - }; - }); + describe('in lib', function() { + var fakeApi = require('./helpers/fake-api.js')(); + before(function() { + fakeApi.get('/notafile', function(req, res, next) { + res.send(404); + next(); + }); + fakeApi.listen(4243); + }) - it('should be handled gracefully with long_poll option while hydrating', function(done) { - this.timeout(10000); + after(function() { + fakeApi.close(); + }); - request(hydrationServer).post('/hydrate') - .send({ - file_path: 'http://anyfetch.com', - callback: 'http://anyfetch.com/result', - document: { - metadata: { - "foo": "bar" - }, - }, - 'long_poll': true - }) - .expect(400) - .expect(/err/i) - .expect(/buggy/i) - .expect(400) - .end(done); - }); + it('should be handled gracefully while hydrating', function(done) { + this.timeout(10000); + request(hydrationServer).post('/hydrate') + .send({ + file_path: 'http://127.0.0.1:4243/afile', + callback: 'http://127.0.0.1:4243/result', + document: { + metadata: { + "foo": "bar" + } + } + }) + .expect(204) + .end(function() {}); - it.only('should be handled gracefully if file does not exists', function(done) { - this.timeout(10000); + hydrationServer.queue.drain = function(err) { + hydrationServer.queue.drain = null; + done(err); + }; + }); - request(hydrationServer).post('/hydrate') - .send({ - file_path: 'http://oseftarace.com/NOPE?some_query', - callback: 'http://oseftarace.com/result?some_query', - document: { - metadata: { - "foo": "bar" + it('should be handled gracefully with long_poll option while hydrating', function(done) { + request(hydrationServer).post('/hydrate') + .send({ + file_path: 'http://127.0.0.1:4243/afile', + callback: 'http://127.0.0.1:4243/result', + document: { + metadata: { + "foo": "bar" + }, }, - }, - 'long_poll': true - }) - .expect(400) - .expect(/err/i) - .expect(/downloading file/i) - .expect(/404/i) - .end(done); + 'long_poll': true + }) + .expect(400) + .expect(/err/i) + .expect(/buggy/i) + .end(done); + }); + + it('should be handled gracefully if file does not exists', function(done) { + this.timeout(10000); + request(hydrationServer).post('/hydrate') + .send({ + file_path: 'http://127.0.0.1:4243/notafile', + callback: 'http://127.0.0.1:4243/result', + document: { + metadata: { + "foo": "bar" + }, + }, + 'long_poll': true + }) + .expect(400) + .expect(/err/i) + .expect(/downloading file/i) + .end(done); + }); }); -}); -describe('hydrationErrors', function() { - var config = { - hydrater_function: __dirname + '/hydraters/errored-hydrater.js', - logger: function() {// Will be pinged with error. We don't care. - } - }; - var hydrationErrorServer = anyfetchFileHydrater.createServer(config); + describe('in hydrators', function() { + var config = { + hydrater_function: __dirname + '/hydraters/errored-hydrater.js', + logger: function() {// Will be pinged with error. We don't care. + } + }; + var hydrationErrorServer = anyfetchFileHydrater.createServer(config); - it('should be handled gracefully while hydrating', function(done) { - this.timeout(10000); + it('should be handled gracefully while hydrating', function(done) { + var fakeApi = require('./helpers/fake-api.js')(); + fakeApi.patch('/result', function(req, res, next) { + //should + res.send(204); + next(); - var fakeApi = restify.createServer(); - fakeApi.use(restify.queryParser()); - fakeApi.use(restify.bodyParser()); + if(req.params.hydration_errored && req.params.hydration_error === "hydrater errored") { + done(); + } + else { + done(new Error("Invalid call")); + } + fakeApi.close(); - fakeApi.patch('/result', function(req, res, next) { - //should - res.send(204); - next(); + }); - if(req.params.hydration_errored && req.params.hydration_error === "hydrater errored") { - done(); - } - else { - done(new Error("Invalid call")); - } - fakeApi.close(); - }); - fakeApi.listen(4242); + fakeApi.listen(4243); - request(hydrationErrorServer).post('/hydrate') - .send({ - file_path: 'http://osef.com/', - callback: 'http://127.0.0.1:4242/result', - document: { - metadata: { - "foo": "bar" + request(hydrationErrorServer).post('/hydrate') + .send({ + file_path: 'http://127.0.0.1:4243/afile', + callback: 'http://127.0.0.1:4243/result', + document: { + metadata: { + "foo": "bar" + } } - } - }) - .expect(202) - .end(function(err) { - if(err) { - throw err; - } - }); + }) + .expect(202) + .end(function(err) { + if(err) { + throw err; + } + }); + }); }); }); diff --git a/test/helpers/fake-api.js b/test/helpers/fake-api.js new file mode 100644 index 0000000..1449528 --- /dev/null +++ b/test/helpers/fake-api.js @@ -0,0 +1,17 @@ +"use strict"; + +var restify = require('restify'); +var fs = require('fs'); + +module.exports = function() { + var fakeApi = restify.createServer(); + fakeApi.use(restify.queryParser()); + fakeApi.use(restify.bodyParser()); + fakeApi.use(restify.acceptParser(fakeApi.acceptable)); + + fakeApi.get('/afile', function(req, res, next) { + fs.createReadStream(__filename, {encoding: 'utf8'}).pipe(res); + next(); + }); + return fakeApi; +} diff --git a/test/helpers/hydrater.js b/test/helpers/hydrater.js index c95ff91..fe5432e 100644 --- a/test/helpers/hydrater.js +++ b/test/helpers/hydrater.js @@ -2,90 +2,100 @@ require('should'); var path = require("path"); +var createFakeApi = require('./fake-api.js') +describe("hydrate()", function() { + var fakeApi = createFakeApi(); + before(function() { + fakeApi.listen(4243); + }); + + after(function() { + fakeApi.close(); + }); -describe("Hydrated document", function() { - it("should have only updated changes", function(done) { - var config = { - hydrater_function: path.resolve(__dirname, '../hydraters/dummy-hydrater.js'), - logger: function(str, err) { - if(err) { - throw err; + describe("Hydrated document", function() { + it("should have only updated changes", function(done) { + var config = { + hydrater_function: path.resolve(__dirname, '../hydraters/dummy-hydrater.js'), + logger: function(str, err) { + if(err) { + throw err; + } } - } - }; - var hydrate = require('../../lib/helpers/hydrater')(config.hydrater_function, config.logger); + }; + var hydrate = require('../../lib/helpers/hydrater')(config.hydrater_function, config.logger); - var task = { - callback: "http://osef.com", - filepath: "/tmp/anyfetch-hydrater.test", - document: { - id: "azerty" - } - }; + var task = { + file_path: "http://127.0.0.1:4243/afile", + callback: "http://127.0.0.1:4243", + document: { + id: "azerty" + } + }; - hydrate(task, function(changes) { - changes.should.have.keys(["metadata"]); - done(); + hydrate(task, function(err, changes) { + changes.should.have.keys(["metadata"]); + done(); + }); }); - }); - it("should keep Dates", function(done) { - var config = { - hydrater_function: path.resolve(__dirname, '../hydraters/update-date-hydrater.js'), - logger: function(str, err) { - if(err) { - throw err; + it("should keep Dates", function(done) { + var config = { + hydrater_function: path.resolve(__dirname, '../hydraters/update-date-hydrater.js'), + logger: function(str, err) { + if(err) { + throw err; + } } - } - }; - var hydrate = require('../../lib/helpers/hydrater')(config.hydrater_function, config.logger); - var task = { - callback: "http://osef.com", - filepath: "/tmp/anyfetch-hydrater.test", - document: { - id: "azerty" - } - }; + }; + var hydrate = require('../../lib/helpers/hydrater')(config.hydrater_function, config.logger); + var task = { + file_path: "http://127.0.0.1:4243/afile", + callback: "http://127.0.0.1:4243", + document: { + id: "azerty" + } + }; - hydrate(task, function(changes) { - changes.should.have.keys(["creation_date"]); - done(); + hydrate(task, function(err, changes) { + changes.should.have.keys(["creation_date"]); + done(); + }); }); }); -}); + describe('Timeout', function() { + var shellFork = require('child_process').fork; + var HydrationError = require('../../lib/index.js').HydrationError; -describe('Timeout', function() { -var shellFork = require('child_process').fork; -var HydrationError = require('../../lib/index.js').HydrationError; + it('should send an error', function(done) { + process.env.TIMEOUT = 20; - it('should send an error', function(done) { - process.env.TIMEOUT = 20; - - var config = { - hydrater_function: path.resolve(__dirname, '../hydraters/too-long-hydrater.js'), - logger: function(str, err) { - if(err) { - throw err; + var config = { + hydrater_function: path.resolve(__dirname, '../hydraters/too-long-hydrater.js'), + logger: function(str, err) { + if(err) { + throw err; + } } - } - }; - var hydrate = require('../../lib/helpers/hydrater.js')(config.hydrater_function, config.logger); + }; + var hydrate = require('../../lib/helpers/hydrater.js')(config.hydrater_function, config.logger); - var task = { - callback: "http://wedontcare.com", - filepath: "/tmp/anyfetch-hydrater.test", - document: { - id: "azerty" - } - }; + var task = { + file_path: "http://127.0.0.1:4243/afile", + callback: "http://127.0.0.1:4243", + document: { + id: "azerty" + } + }; - hydrate(task, function(changes) { - changes.should.have.property('hydration_errored', true); - process.env.TIMEOUT = 60 * 1000; - done(); + hydrate(task, function(err, changes) { + changes.should.have.property('hydration_errored', true); + process.env.TIMEOUT = 60 * 1000; + done(); + }); }); }); }); diff --git a/test/hook.js b/test/hook.js index 173fc78..83143f7 100644 --- a/test/hook.js +++ b/test/hook.js @@ -3,81 +3,49 @@ require('should'); var request = require('supertest'); var restify = require('restify'); -var fs = require('fs'); - - +var fs = require("fs"); var anyfetchFileHydrater = require('../lib/'); +var createFakeApi = require('./helpers/fake-api.js'); -describe.skip('/hydrate webhooks', function() { - // Patch AnyFetch URL - // Avoid uselessly pinging anyfetch.com with invalid requests - process.env.ANYFETCH_API_URL = 'http://localhost'; - - var dummyHydrater; - var hydrationFunction = function(path, document, changes, cb) { - dummyHydrater(path, document, changes, cb); - }; - var config = { - hydrater_function: hydrationFunction, - logger: function(str, err) { - if(err) { - throw err; - } - } - }; - var hydrationServer = anyfetchFileHydrater.createServer(config); - - // Create a fake HTTP server to send a file and test results - var fileServer = restify.createServer(); - fileServer.use(restify.acceptParser(fileServer.acceptable)); - fileServer.use(restify.queryParser()); - fileServer.use(restify.bodyParser()); - - fileServer.get('/file', function(req, res, next) { - fs.createReadStream(__filename).pipe(res); - next(); - }); - fileServer.listen(1337); - - var patchFunction; - fileServer.patch('/result', function(req, res, next) { - patchFunction(req, res, next); - }); - +describe('/hydrate webhooks', function() { it('should be pinged with hydrater result', function(done) { - dummyHydrater = function(path, document, changes, cb) { - if(document.replace) { - return cb(); + var config = { + hydrater_function: __dirname + '/hydraters/useful-hydrater.js', + logger: function(str, err) { + if(err) { + throw err; + } } - - changes.metadata.path = path; - changes.metadata.text = fs.readFileSync(path).toString(); - - cb(null, changes); }; + var hydrationServer = anyfetchFileHydrater.createServer(config); - patchFunction = function(req, res, next) { + var fakeApi = createFakeApi(); + fakeApi.patch('/result', function(req, res, next) { try { req.params.should.have.property('metadata'); req.params.metadata.should.not.have.property('foo'); req.params.metadata.should.have.property('path'); - req.params.metadata.should.have.property('text', fs.readFileSync(__filename).toString()); + req.params.metadata.should.have.property('text', fs.readFileSync(__dirname + '/helpers/fake-api.js').toString()); res.send(204); next(); done(); + fakeApi.close(); } catch(e) { // We need a try catch cause mocha is try-catching on main event loop, and the server create a new stack. done(e); + fakeApi.close(); } - }; + }); + + fakeApi.listen(4243); request(hydrationServer).post('/hydrate') .send({ - file_path: 'http://127.0.0.1:1337/file', - callback: 'http://127.0.0.1:1337/result', + file_path: 'http://127.0.0.1:4243/afile', + callback: 'http://127.0.0.1:4243/result', document: { metadata: { "foo": "bar" @@ -89,18 +57,28 @@ describe.skip('/hydrate webhooks', function() { }); it('should not be pinged on skipped task', function(done) { - dummyHydrater = function(path, document, changes, cb) { - return cb(null, null); + var config = { + hydrater_function: __dirname + '/hydraters/skipper-hydrater.js', + logger: function(str, err) { + if(err) { + throw err; + } + } }; + var hydrationServer = anyfetchFileHydrater.createServer(config); - patchFunction = function() { + var fakeApi = createFakeApi(); + fakeApi.patch('/result', function(req, res, next) { done(new Error("should not be called")); - }; + fakeApi.close(); + }); + + fakeApi.listen(4243); request(hydrationServer).post('/hydrate') .send({ - file_path: 'http://127.0.0.1:1337/file', - callback: 'http://127.0.0.1:1337/result', + file_path: 'http://127.0.0.1:4243/afile', + callback: 'http://127.0.0.1:4243/result', document: { metadata: { "foo": "bar" @@ -113,13 +91,9 @@ describe.skip('/hydrate webhooks', function() { var interval = setInterval(function() { if(hydrationServer.queue.length() === 0) { clearInterval(interval); + fakeApi.close(); done(); } }, 25); - - }); - - after(function() { - fileServer.close(); }); }); diff --git a/test/hydraters/skipper-hydrater.js b/test/hydraters/skipper-hydrater.js new file mode 100644 index 0000000..94587f4 --- /dev/null +++ b/test/hydraters/skipper-hydrater.js @@ -0,0 +1,3 @@ +module.exports = function dummyHydrater(path, document, changes, cb) { + return cb(null, null); +}; diff --git a/test/hydraters/useful-hydrater.js b/test/hydraters/useful-hydrater.js new file mode 100644 index 0000000..4dbe998 --- /dev/null +++ b/test/hydraters/useful-hydrater.js @@ -0,0 +1,7 @@ +var fs = require("fs"); + +module.exports = function usefulHydrater(path, document, changes, cb) { + changes.metadata.path = path; + changes.metadata.text = fs.readFileSync(path).toString(); + cb(null, changes); +}; \ No newline at end of file diff --git a/test/index.js b/test/index.js index 765bc87..9cd8591 100644 --- a/test/index.js +++ b/test/index.js @@ -15,13 +15,26 @@ var config = { }; describe('POST /hydrate', function() { + var fakeApi = require('./helpers/fake-api.js')(); + before(function() { + + fakeApi.patch('/callback', function(req, res, next) { + res.send(204); + next(); + }); + fakeApi.listen(4243); + }) + + after(function() { + fakeApi.close(); + }); var server = anyfetchFileHydrater.createServer(config); it('should refuse request without callback', function(done) { request(server).post('/hydrate') .send({ - 'file_path': 'http://anyfetch.com/file', + 'file_path': 'http://127.0.0.1:4243/afile', 'document': { 'metadata': {}, } @@ -33,7 +46,7 @@ describe('POST /hydrate', function() { it('should accept request without callback when long_polling', function(done) { request(server).post('/hydrate') .send({ - 'file_path': 'http://anyfetch.com', + 'file_path': 'http://127.0.0.1:4243/afile', 'long_poll': true, 'document': { 'metadata': {}, @@ -48,8 +61,8 @@ describe('POST /hydrate', function() { request(server) .post('/hydrate') .send({ - 'file_path': 'http://anyfetch.com/', - 'callback': 'http://anyfetch.com/callback', + 'file_path': 'http://127.0.0.1:4243/afile', + 'callback': 'http://127.0.0.1:4243/callback', 'document': { 'metadata': {}, } @@ -64,8 +77,8 @@ describe('POST /hydrate', function() { request(server) .post('/hydrate') .send({ - 'file_path': 'http://anyfetch.com/', - 'callback': 'http://anyfetch.com/callback', + 'file_path': 'http://127.0.0.1:4243/afile', + 'callback': 'http://127.0.0.1:4243/callback', 'long_poll': true, 'document': { 'metadata': {}, From df1ad71d54b821ca30e8b9a788457854dda6cbd9 Mon Sep 17 00:00:00 2001 From: Amoki Date: Wed, 27 Aug 2014 15:01:29 +0200 Subject: [PATCH 3/7] really litle improvment --- lib/helpers/hydrater.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/helpers/hydrater.js b/lib/helpers/hydrater.js index 76f9295..1ddb7b0 100644 --- a/lib/helpers/hydrater.js +++ b/lib/helpers/hydrater.js @@ -88,8 +88,8 @@ module.exports = function(hydraterFunction, logger, errLogger) { }; cleaner.called = false; - child.on('error', function(errCode) { - cleaner(new HydrationError("Wild error appeared during spwaning child. err code:" + errCode)); + child.on('error', function(exitCode) { + cleaner(new HydrationError("Wild error appeared during spwaning child. Exit code:" + exitCode)); }); child.stderr.on('readable', function() { From c2b27c0466a3f6c493146e1c5f3baa50ce56faf1 Mon Sep 17 00:00:00 2001 From: Amoki Date: Wed, 27 Aug 2014 15:08:14 +0200 Subject: [PATCH 4/7] fix all lint errors --- test/errors.js | 4 +--- test/helpers/fake-api.js | 2 +- test/helpers/hydrater.js | 6 +----- test/hook.js | 2 +- test/hydraters/buggy-hydrater.js | 4 +++- test/hydraters/dummy-hydrater.js | 2 ++ test/hydraters/errored-hydrater.js | 5 +++-- test/hydraters/skipper-hydrater.js | 2 ++ test/hydraters/too-long-hydrater.js | 4 +++- test/hydraters/update-date-hydrater.js | 2 ++ test/hydraters/useful-hydrater.js | 2 ++ test/index.js | 2 +- 12 files changed, 22 insertions(+), 15 deletions(-) diff --git a/test/errors.js b/test/errors.js index 3a11330..d5bfa63 100644 --- a/test/errors.js +++ b/test/errors.js @@ -2,8 +2,6 @@ require('should'); var request = require('supertest'); -var restify = require('restify'); -var fs = require("fs"); var anyfetchFileHydrater = require('../lib/'); describe('Errors', function() { @@ -25,7 +23,7 @@ describe('Errors', function() { next(); }); fakeApi.listen(4243); - }) + }); after(function() { fakeApi.close(); diff --git a/test/helpers/fake-api.js b/test/helpers/fake-api.js index 1449528..c8e0440 100644 --- a/test/helpers/fake-api.js +++ b/test/helpers/fake-api.js @@ -14,4 +14,4 @@ module.exports = function() { next(); }); return fakeApi; -} +}; diff --git a/test/helpers/hydrater.js b/test/helpers/hydrater.js index fe5432e..8e203f4 100644 --- a/test/helpers/hydrater.js +++ b/test/helpers/hydrater.js @@ -2,7 +2,7 @@ require('should'); var path = require("path"); -var createFakeApi = require('./fake-api.js') +var createFakeApi = require('./fake-api.js'); describe("hydrate()", function() { var fakeApi = createFakeApi(); @@ -66,10 +66,6 @@ describe("hydrate()", function() { }); describe('Timeout', function() { - var shellFork = require('child_process').fork; - var HydrationError = require('../../lib/index.js').HydrationError; - - it('should send an error', function(done) { process.env.TIMEOUT = 20; diff --git a/test/hook.js b/test/hook.js index 83143f7..75cd106 100644 --- a/test/hook.js +++ b/test/hook.js @@ -2,7 +2,6 @@ require('should'); var request = require('supertest'); -var restify = require('restify'); var fs = require("fs"); var anyfetchFileHydrater = require('../lib/'); var createFakeApi = require('./helpers/fake-api.js'); @@ -70,6 +69,7 @@ describe('/hydrate webhooks', function() { var fakeApi = createFakeApi(); fakeApi.patch('/result', function(req, res, next) { done(new Error("should not be called")); + next(); fakeApi.close(); }); diff --git a/test/hydraters/buggy-hydrater.js b/test/hydraters/buggy-hydrater.js index 9b0e381..6d8f6be 100644 --- a/test/hydraters/buggy-hydrater.js +++ b/test/hydraters/buggy-hydrater.js @@ -1,3 +1,5 @@ -module.exports = function buggyHydrater(path, document, changes, cb) { +"use strict"; + +module.exports = function buggyHydrater() { throw new Error("I'm buggy"); }; diff --git a/test/hydraters/dummy-hydrater.js b/test/hydraters/dummy-hydrater.js index 86d1fee..41eab75 100644 --- a/test/hydraters/dummy-hydrater.js +++ b/test/hydraters/dummy-hydrater.js @@ -1,3 +1,5 @@ +"use strict"; + module.exports = function dummyHydrater(path, document, changes, cb) { changes.metadata.hydrated = true; cb(null, changes); diff --git a/test/hydraters/errored-hydrater.js b/test/hydraters/errored-hydrater.js index 312651b..95c5dad 100644 --- a/test/hydraters/errored-hydrater.js +++ b/test/hydraters/errored-hydrater.js @@ -1,6 +1,7 @@ -var anyfetchFileHydrater = require('../../lib/'); +"use strict"; +var anyfetchFileHydrater = require('../../lib/'); module.exports = function erroredHydrater(path, document, changes, cb) { - cb(new anyfetchFileHydrater.HydrationError("hydrater errored")) + cb(new anyfetchFileHydrater.HydrationError("hydrater errored")); }; diff --git a/test/hydraters/skipper-hydrater.js b/test/hydraters/skipper-hydrater.js index 94587f4..1405f1d 100644 --- a/test/hydraters/skipper-hydrater.js +++ b/test/hydraters/skipper-hydrater.js @@ -1,3 +1,5 @@ +"use strict"; + module.exports = function dummyHydrater(path, document, changes, cb) { return cb(null, null); }; diff --git a/test/hydraters/too-long-hydrater.js b/test/hydraters/too-long-hydrater.js index a4b1849..41f8a3d 100644 --- a/test/hydraters/too-long-hydrater.js +++ b/test/hydraters/too-long-hydrater.js @@ -1,6 +1,8 @@ +"use strict"; + module.exports = function tooLongHydrater(path, document, changes, cb) { setTimeout(function() { changes.changed = true; cb(null, changes); - }, 1000) + }, 1000); }; diff --git a/test/hydraters/update-date-hydrater.js b/test/hydraters/update-date-hydrater.js index 4ebda1b..e80decd 100644 --- a/test/hydraters/update-date-hydrater.js +++ b/test/hydraters/update-date-hydrater.js @@ -1,3 +1,5 @@ +"use strict"; + module.exports = function updateDateHydrater(path, document, changes, cb) { changes.creation_date = new Date(); cb(null, changes); diff --git a/test/hydraters/useful-hydrater.js b/test/hydraters/useful-hydrater.js index 4dbe998..514c939 100644 --- a/test/hydraters/useful-hydrater.js +++ b/test/hydraters/useful-hydrater.js @@ -1,3 +1,5 @@ +"use strict"; + var fs = require("fs"); module.exports = function usefulHydrater(path, document, changes, cb) { diff --git a/test/index.js b/test/index.js index 9cd8591..917392d 100644 --- a/test/index.js +++ b/test/index.js @@ -23,7 +23,7 @@ describe('POST /hydrate', function() { next(); }); fakeApi.listen(4243); - }) + }); after(function() { fakeApi.close(); From 2571c7c4786ba83da2117ab799f50d3285f82595 Mon Sep 17 00:00:00 2001 From: Amoki Date: Wed, 27 Aug 2014 15:09:20 +0200 Subject: [PATCH 5/7] typo --- test/hydraters/useful-hydrater.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hydraters/useful-hydrater.js b/test/hydraters/useful-hydrater.js index 514c939..9cbf514 100644 --- a/test/hydraters/useful-hydrater.js +++ b/test/hydraters/useful-hydrater.js @@ -6,4 +6,4 @@ module.exports = function usefulHydrater(path, document, changes, cb) { changes.metadata.path = path; changes.metadata.text = fs.readFileSync(path).toString(); cb(null, changes); -}; \ No newline at end of file +}; From 3f4c49a20cda7bce8a71e7c62c6b740f48cac9ec Mon Sep 17 00:00:00 2001 From: Amoki Date: Wed, 27 Aug 2014 16:31:31 +0200 Subject: [PATCH 6/7] some improvments --- lib/helpers/hydrater.js | 17 +++++++++-------- test/errors.js | 1 - test/helpers/fake-api.js | 2 +- test/helpers/hydrater.js | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/helpers/hydrater.js b/lib/helpers/hydrater.js index 1ddb7b0..43280fc 100644 --- a/lib/helpers/hydrater.js +++ b/lib/helpers/hydrater.js @@ -54,7 +54,7 @@ module.exports = function(hydraterFunction, logger, errLogger) { if(err) { err = new restify.BadGatewayError('Error when downloading file ' + task.file_path + ': ' + err); } - cb(err, res.text); + cb(err, res && res.text); }); } else { @@ -65,7 +65,7 @@ module.exports = function(hydraterFunction, logger, errLogger) { if(res) { fs.writeFile(path, res, cb); } - else{ + else { cb(); } }, @@ -81,7 +81,6 @@ module.exports = function(hydraterFunction, logger, errLogger) { var cleaner = function(err, changes) { if(!cleaner.called) { cleaner.called = true; - child.kill('SIGKILL'); cb(err, changes); } clearTimeout(timeout); @@ -89,7 +88,7 @@ module.exports = function(hydraterFunction, logger, errLogger) { cleaner.called = false; child.on('error', function(exitCode) { - cleaner(new HydrationError("Wild error appeared during spwaning child. Exit code:" + exitCode)); + cleaner(new HydrationError("Wild error appeared while spawning child. Exit code:" + exitCode)); }); child.stderr.on('readable', function() { @@ -108,7 +107,7 @@ module.exports = function(hydraterFunction, logger, errLogger) { child.on('exit', function(errCode) { if(errCode !== 0) { - cleaner(new HydrationError(["Child exiting with err code: " + errCode, stdout, stderr])); + cleaner(new HydrationError("Child exiting with err code: " + errCode + stdout + stderr)); } }); @@ -130,15 +129,16 @@ module.exports = function(hydraterFunction, logger, errLogger) { }); child.on('message', function(res) { + var err = res.err // If the function replied with an "HydrationError", we'll wrap this in a nicely formatted document // and stop the error from bubbling up. - if(res.err && res.err._hydrationError === true) { + if(err && err._hydrationError) { res.changes = {}; res.changes.hydration_errored = true; res.changes.hydration_error = res.err.message; - res.err = null; + err = null; } - cleaner(res.err, res.changes); + cleaner(err, res.changes); }); @@ -147,6 +147,7 @@ module.exports = function(hydraterFunction, logger, errLogger) { var changes = {}; changes.hydration_errored = true; changes.hydration_error = "Task took too long."; + child.kill('SIGKILL'); cleaner(null, changes); } }, process.env.TIMEOUT || 60 * 1000); diff --git a/test/errors.js b/test/errors.js index d5bfa63..d8e1589 100644 --- a/test/errors.js +++ b/test/errors.js @@ -99,7 +99,6 @@ describe('Errors', function() { it('should be handled gracefully while hydrating', function(done) { var fakeApi = require('./helpers/fake-api.js')(); fakeApi.patch('/result', function(req, res, next) { - //should res.send(204); next(); diff --git a/test/helpers/fake-api.js b/test/helpers/fake-api.js index c8e0440..187914e 100644 --- a/test/helpers/fake-api.js +++ b/test/helpers/fake-api.js @@ -10,7 +10,7 @@ module.exports = function() { fakeApi.use(restify.acceptParser(fakeApi.acceptable)); fakeApi.get('/afile', function(req, res, next) { - fs.createReadStream(__filename, {encoding: 'utf8'}).pipe(res); + fs.createReadStream(__filename).pipe(res); next(); }); return fakeApi; diff --git a/test/helpers/hydrater.js b/test/helpers/hydrater.js index 8e203f4..b5b1491 100644 --- a/test/helpers/hydrater.js +++ b/test/helpers/hydrater.js @@ -90,7 +90,7 @@ describe("hydrate()", function() { hydrate(task, function(err, changes) { changes.should.have.property('hydration_errored', true); process.env.TIMEOUT = 60 * 1000; - done(); + done(err); }); }); }); From 4870fd0e41bee96e4e0004258c0b680953431570 Mon Sep 17 00:00:00 2001 From: Amoki Date: Wed, 27 Aug 2014 16:34:02 +0200 Subject: [PATCH 7/7] better check on timeout test --- test/helpers/hydrater.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/helpers/hydrater.js b/test/helpers/hydrater.js index b5b1491..ddc70ae 100644 --- a/test/helpers/hydrater.js +++ b/test/helpers/hydrater.js @@ -89,6 +89,7 @@ describe("hydrate()", function() { hydrate(task, function(err, changes) { changes.should.have.property('hydration_errored', true); + changes.should.have.property('hydration_error', 'Task took too long.'); process.env.TIMEOUT = 60 * 1000; done(err); });