From 23dae97db7127942f252346ee9b1d7e4e6aa2926 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 26 Nov 2015 08:11:54 +0800 Subject: [PATCH 01/18] Change function invocation to pass additional shortHash argument --- lib/models/api.js | 44 ++++++++++++++++++++++++++-------------- lib/models/error-page.js | 2 +- lib/models/mongo.js | 7 +++++-- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/lib/models/api.js b/lib/models/api.js index 74a1a74..f204006 100644 --- a/lib/models/api.js +++ b/lib/models/api.js @@ -113,12 +113,18 @@ api._shouldBypassAuth = function (req) { /** * Proxy request to detention if container is not running. * Proxy request to container is running + * @param {Object} targetNaviEntryInstance + * @param {String} shortHash + * @param {String} reqUrl + * @param {Object} req + * @param {Function} next */ -api._processTargetInstance = function (targetNaviEntryInstance, reqUrl, req, next) { +api._processTargetInstance = function (targetNaviEntryInstance, shortHash, reqUrl, req, next) { var logData = { tx: true, targetNaviEntryInstance: targetNaviEntryInstance, - reqUrl: reqUrl + reqUrl: reqUrl, + shortHash: shortHash }; log.info(logData, 'api._processTargetInstance'); @@ -129,12 +135,11 @@ api._processTargetInstance = function (targetNaviEntryInstance, reqUrl, req, nex // for error-page module if container is unresponsive req.targetBranch = targetNaviEntryInstance.branch; - if (!targetNaviEntryInstance.running) { log.trace(logData, '_processTargetInstance !running'); req.targetHost = errorPage.generateErrorUrl('not_running', { elasticUrl: reqUrl, - targetBranch: targetNaviEntryInstance.branch + shortHash: shortHash }); return next(); } @@ -192,29 +197,33 @@ api._getTargetHostElastic = function (logData, req, next) { // proxy to user-mapped instance (or master if no user mapping) log.trace(logData, 'getTargetHost redis.lrange mongo.fetchNaviEntry isBrowser !refererUrl'); // document might not have a userMappings key yet - var mappedInstanceId = (naviEntry.userMappings) ? + var mappedInstanceShortHash = (naviEntry.userMappings) ? naviEntry.userMappings[req.session.userId] : null; - if (!mappedInstanceId) { + if (!mappedInstanceShortHash) { log.trace(logData, 'getTargetHost redis.lrange mongo.fetchNaviEntry isBrowser !refererUrl '+ - '!mappedInstanceId'); + '!mappedInstanceShortHash'); // use master targetNaviEntryInstance = mongo.constructor.findMasterPodBranch(naviEntry); } else { log.trace(put({ - mappedInstanceId: mappedInstanceId + mappedInstanceShortHash: mappedInstanceShortHash }, logData), 'getTargetHost redis.lrange mongo.fetchNaviEntry isBrowser !refererUrl '+ - 'mappedInstanceId'); - targetNaviEntryInstance = naviEntry.directUrls[mappedInstanceId]; + 'mappedInstanceShortHash'); + targetNaviEntryInstance = naviEntry.directUrls[mappedInstanceShortHash]; } - return api._processTargetInstance(targetNaviEntryInstance, reqUrl, req, next); + return api._processTargetInstance(targetNaviEntryInstance, mappedInstanceShortHash, + reqUrl, req, next); } } else { // if not browser, proxy to master log.trace(logData, 'getTargetHost redis.lrange mongo.fetchNaviEntry !isBrowser'); - targetNaviEntryInstance = mongo.constructor.findMasterPodBranch(naviEntry); - return api._processTargetInstance(targetNaviEntryInstance, reqUrl, req, next); + var findResult = mongo.constructor.findMasterPodBranch(naviEntry); + targetNaviEntryInstance = findResult.directUrlObj; + return api._processTargetInstance(targetNaviEntryInstance, + findResult.directUrlShortHash, + reqUrl, req, next); } }); }; @@ -295,9 +304,14 @@ api._getTargetHostElasticReferer = function (logData, naviEntry, req, next) { }, logData), 'getTargetHost redis.lrange mongo.fetchNaviEntry isBrowser !targetNaviEntryInstance '+ 'use master'); - targetNaviEntryInstance = mongo.constructor.findMasterPodBranch(naviEntry); + var findResult = mongo.constructor.findMasterPodBranch(naviEntry); + targetNaviEntryInstance = findResult.directUrlObj; + return api._processTargetInstance(targetNaviEntryInstance, + findResult.directUrlShortHash, + reqUrl, req, next); + } - return api._processTargetInstance(targetNaviEntryInstance, reqUrl, req, next); + return api._processTargetInstance(targetNaviEntryInstance, instanceShortHash, reqUrl, req, next); }; /** diff --git a/lib/models/error-page.js b/lib/models/error-page.js index 2d647d3..3d73fbd 100644 --- a/lib/models/error-page.js +++ b/lib/models/error-page.js @@ -40,7 +40,7 @@ function generateErrorUrl(type, opts) { log.trace(logData, 'generateErrorUrl signin'); } else if (type === 'not_running' || type === 'unresponsive' || type === 'ports') { query.elasticUrl = opts.elasticUrl; - query.targetBranch = opts.targetBranch; + query.shortHash = opts.shortHash; } else { throw ErrorCat.createAndReport(500, 'invalid error page'); } diff --git a/lib/models/mongo.js b/lib/models/mongo.js index 8eec403..2d3f636 100644 --- a/lib/models/mongo.js +++ b/lib/models/mongo.js @@ -135,10 +135,13 @@ Mongo.findMasterPodBranch = function (naviEntry) { for (var i = 0, len = shortHashes.length; i < len; i++) { var directUrlObj = naviEntry.directUrls[shortHashes[i]]; if (directUrlObj.masterPod) { - return directUrlObj; + return { + directUrlShortHash: shortHashes[i], + directUrlObj: directUrlObj + }; } } - log.warn(logData, 'findMasterPodBranch !match'); + log.error(logData, 'findMasterPodBranch !match'); return null; }; From 901b991355fc174bf76a0b46fa8bfd980eb47199 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 26 Nov 2015 08:18:34 +0800 Subject: [PATCH 02/18] Fix reference errors in api module --- lib/models/api.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/models/api.js b/lib/models/api.js index f204006..7a3ec01 100644 --- a/lib/models/api.js +++ b/lib/models/api.js @@ -204,7 +204,9 @@ api._getTargetHostElastic = function (logData, req, next) { 'getTargetHost redis.lrange mongo.fetchNaviEntry isBrowser !refererUrl '+ '!mappedInstanceShortHash'); // use master - targetNaviEntryInstance = mongo.constructor.findMasterPodBranch(naviEntry); + var findResult = mongo.constructor.findMasterPodBranch(naviEntry); + targetNaviEntryInstance = findResult.directUrlObj; + mappedInstanceShortHash = findResult.directUrlShortHash; } else { log.trace(put({ mappedInstanceShortHash: mappedInstanceShortHash From 36bca3db2c84b0f3c37d0a05f6f539a3e4ae07c3 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 26 Nov 2015 08:21:38 +0800 Subject: [PATCH 03/18] Update unit test stubs and expects --- test/unit/api.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/api.js b/test/unit/api.js index f0f8627..cc966fd 100644 --- a/test/unit/api.js +++ b/test/unit/api.js @@ -220,7 +220,7 @@ describe('api.js unit test', function () { describe('_processTargetInstance', function () { it('should next with error if !targetNaviEntryInstance', function (done) { - api._processTargetInstance(null, '', {}, function (err) { + api._processTargetInstance(null, '', '', {}, function (err) { expect(err.message).to.equal('Not Found'); done(); }); @@ -232,10 +232,10 @@ describe('api.js unit test', function () { api._processTargetInstance({ running: false, branch: 'master' - }, reqUrl, req, function (err) { + }, '55555', reqUrl, req, function (err) { expect(err).to.be.undefined(); expect(req.targetHost).to.equal('http://localhost:55551?type=not_running&elasticUrl='+ - reqUrl+'&targetBranch=master'); + reqUrl+'&shortHash=55555'); done(); }); }); From 60a08df6865b30eb48a2cec5a26269c93b1fad93 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 26 Nov 2015 08:27:39 +0800 Subject: [PATCH 04/18] Fix referer entry masterPod lookup --- lib/models/api.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/models/api.js b/lib/models/api.js index 7a3ec01..9b535f7 100644 --- a/lib/models/api.js +++ b/lib/models/api.js @@ -262,8 +262,9 @@ api._getTargetHostElasticReferer = function (logData, naviEntry, req, next) { var refererNaviEntryInstance; if (!refererUserMappedInstanceId) { // no user-mapping for current user and referer instance, use master - refererNaviEntryInstance = - mongo.constructor.findMasterPodBranch(naviEntry.refererNaviEntry); + var findResult = mongo.constructor.findMasterPodBranch(naviEntry.refererNaviEntry); + refererNaviEntryInstance = keypather.get(findResult, 'directUrlObj'); + refererUserMappedInstanceId = keypather.get(findResult, 'directUrlShortHash'); log.trace(put({ refererUserMappedInstanceId: refererUserMappedInstanceId, refererNaviEntryInstance: refererNaviEntryInstance From 9d8369aca7c64521e4ec1ff0a2f172914b4e1ffa Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 26 Nov 2015 08:29:33 +0800 Subject: [PATCH 05/18] Update error-page unit test expectations --- test/unit/error-page.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/unit/error-page.js b/test/unit/error-page.js index 9b4d5a6..89d8dfe 100644 --- a/test/unit/error-page.js +++ b/test/unit/error-page.js @@ -29,39 +29,39 @@ describe('error-page.js unit test', function () { it('should generate error url for not-running error', function (done) { var proxyUrl = errorPage.generateErrorUrl('not_running', { elasticUrl: 'api-staging-codenow.runnableapp.com', - targetBranch: 'master' + shortHash: '555' }); expect(proxyUrl).to .equal( 'http://localhost:55551?'+ 'type=not_running&elasticUrl=api-staging-codenow.runnableapp.com'+ - '&targetBranch=master'); + '&shortHash=555'); done(); }); it('should generate error url for unresponsive error', function (done) { var proxyUrl = errorPage.generateErrorUrl('unresponsive', { elasticUrl: 'api-staging-codenow.runnableapp.com', - targetBranch: 'master' + shortHash: '555' }); expect(proxyUrl).to .equal( 'http://localhost:55551?'+ 'type=unresponsive&elasticUrl=api-staging-codenow.runnableapp.com'+ - '&targetBranch=master'); + '&shortHash=555'); done(); }); it('should generate error url for ports error', function (done) { var proxyUrl = errorPage.generateErrorUrl('ports', { elasticUrl: 'api-staging-codenow.runnableapp.com', - targetBranch: 'master' + shortHash: '555' }); expect(proxyUrl).to .equal( 'http://localhost:55551?'+ 'type=ports&elasticUrl=api-staging-codenow.runnableapp.com'+ - '&targetBranch=master'); + '&shortHash=555'); done(); }); @@ -69,7 +69,7 @@ describe('error-page.js unit test', function () { function throws () { errorPage.generateErrorUrl('skjfasghasdg', { elasticUrl: 'api-staging-codenow.runnableapp.com', - targetBranch: 'master' + shortHash: '555' }); } expect(throws).to.throw(); From c23a0eb8082f0ec8b3a55268f66a293a19a51e62 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 26 Nov 2015 08:30:32 +0800 Subject: [PATCH 06/18] Update mongo unit test expects --- test/unit/mongo.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/mongo.js b/test/unit/mongo.js index 9b198e6..8115dff 100644 --- a/test/unit/mongo.js +++ b/test/unit/mongo.js @@ -232,7 +232,8 @@ describe('lib/models/mongodb', function () { var copy = put({}, naviEntryFixtures); copy.directUrls.e4rov2.masterPod = false; copy.directUrls.e4v7ve.masterPod = true; - var masterPod = mongo.constructor.findMasterPodBranch(copy); + var findResult = mongo.constructor.findMasterPodBranch(copy); + var masterPod = findResult.directUrlObj; expect(masterPod.masterPod).to.equal(true); expect(masterPod).to.equal(copy.directUrls.e4v7ve); done(); From 52ad0142763fbbda19977a3f36c64bfeb93d6b92 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Mon, 30 Nov 2015 15:53:37 +0800 Subject: [PATCH 07/18] Add timing debug logs --- lib/models/proxy.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index fca6e4c..81cea8e 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -165,10 +165,16 @@ Proxy.prototype._streamRes = function (targetRes, proxiedRes, res) { log.trace(logData, '_streamRes resIsHtml'); delete targetRes.headers['content-length']; var scriptInjectStream = scriptInjectResStreamFactory.create(xhrPatchScript, resIsGziped); - proxiedRes - .pipe(scriptInjectStream.input); - scriptInjectStream.output - .pipe(res); + proxiedRes.pipe(scriptInjectStream.input); + + // Added these logs to debug timing of script injection + var stream = scriptInjectStream.output.pipe(res); + stream.on('finish', function () { + log.trace(logData, '_streamRes resIsHtml stream finish'); + }); + stream.on('error', function () { + log.trace(logData, '_streamRes resIsHtml stream error'); + }); } else { log.trace(logData, '_streamRes !resIsHtml'); From 8dff5a098578876d74c85c0258453cc7c5a27ea7 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Mon, 30 Nov 2015 16:03:23 +0800 Subject: [PATCH 08/18] Add properties to req object for erro page generation --- lib/models/api.js | 4 ++++ lib/models/proxy.js | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/models/api.js b/lib/models/api.js index 9b535f7..28823f4 100644 --- a/lib/models/api.js +++ b/lib/models/api.js @@ -133,6 +133,10 @@ api._processTargetInstance = function (targetNaviEntryInstance, shortHash, reqUr return next(ErrorCat.create(404, 'Not Found')); } + // Adding these to req object to make values available in proxy module's error event handler. + req.shortHash = shortHash; + req.elasticUrl = reqUrl; + // for error-page module if container is unresponsive req.targetBranch = targetNaviEntryInstance.branch; if (!targetNaviEntryInstance.running) { diff --git a/lib/models/proxy.js b/lib/models/proxy.js index 81cea8e..76b7321 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -47,8 +47,8 @@ function Proxy () { if (!req.didError) { req.didError = true; var errorUrl = errorPage.generateErrorUrl('unresponsive', { - elasticUrl: '', - targetBranch: '' + shortHash: req.shortHash, + elasticUrl: req.elasticUrl }); var targetHost = getHostAndAugmentReq(req, errorUrl); log.trace(put({ From 0093ccc5c32dabe10c5eb7b947d3e3b4da008009 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Mon, 30 Nov 2015 16:06:53 +0800 Subject: [PATCH 09/18] Add comment for Anand review --- lib/models/proxy.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index 76b7321..ec6d2e5 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -35,7 +35,6 @@ function Proxy () { log.info('Proxy constructor'); var self = this; this.proxy = httpProxy.createProxyServer({}); - // setup error handling this.proxy.on('error', function (err, req/*, proxiedRes */) { var res = req.res; var logData = { @@ -44,6 +43,10 @@ function Proxy () { req: req }; log.error(logData, 'proxy.on error'); + /** + * I am not sure what the original intended purpose of this logic was. I think I should delete + * the req.didError conditional check and set. + */ if (!req.didError) { req.didError = true; var errorUrl = errorPage.generateErrorUrl('unresponsive', { From 85438b39099d77c7c0d14767e789a63c5497383e Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Mon, 30 Nov 2015 16:07:15 +0800 Subject: [PATCH 10/18] Tweak code comment --- lib/models/proxy.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index ec6d2e5..c01fddd 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -44,6 +44,7 @@ function Proxy () { }; log.error(logData, 'proxy.on error'); /** + * Question, Anand: * I am not sure what the original intended purpose of this logic was. I think I should delete * the req.didError conditional check and set. */ From 689977ab00bb95ea2991c182c3b9255d3613ec27 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Mon, 30 Nov 2015 16:09:12 +0800 Subject: [PATCH 11/18] Update trace log metadata --- lib/models/proxy.js | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index c01fddd..19c2f5e 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -42,27 +42,16 @@ function Proxy () { err: err, req: req }; - log.error(logData, 'proxy.on error'); - /** - * Question, Anand: - * I am not sure what the original intended purpose of this logic was. I think I should delete - * the req.didError conditional check and set. - */ - if (!req.didError) { - req.didError = true; - var errorUrl = errorPage.generateErrorUrl('unresponsive', { - shortHash: req.shortHash, - elasticUrl: req.elasticUrl - }); - var targetHost = getHostAndAugmentReq(req, errorUrl); - log.trace(put({ - targetHost: targetHost, - errorUrl: errorUrl - }, logData), 'proxy.on error !req.didError'); - self.proxy.web(req, res, { target: targetHost }); - } else { - log.trace(logData, 'proxy.on error req.didError'); - } + var errorUrl = errorPage.generateErrorUrl('unresponsive', { + shortHash: req.shortHash, + elasticUrl: req.elasticUrl + }); + var targetHost = getHostAndAugmentReq(req, errorUrl); + log.trace(put({ + targetHost: targetHost, + errorUrl: errorUrl + }, logData), 'proxy.on error'); + self.proxy.web(req, res, { target: targetHost }); }); /** From a78789bee7c688dc871d4caeef3ef0e068fa56c9 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Mon, 30 Nov 2015 16:16:53 +0800 Subject: [PATCH 12/18] Remove timing logs --- lib/models/proxy.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index 19c2f5e..17bb1b8 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -161,15 +161,8 @@ Proxy.prototype._streamRes = function (targetRes, proxiedRes, res) { proxiedRes.pipe(scriptInjectStream.input); // Added these logs to debug timing of script injection - var stream = scriptInjectStream.output.pipe(res); - stream.on('finish', function () { - log.trace(logData, '_streamRes resIsHtml stream finish'); - }); - stream.on('error', function () { - log.trace(logData, '_streamRes resIsHtml stream error'); - }); - } - else { + scriptInjectStream.output.pipe(res); + } else { log.trace(logData, '_streamRes !resIsHtml'); // if the response type is not html transformRes should not modify the response // finally pipe target response data to the real response From c1f756b2de363aac423bb37beef60853b8f87079 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Wed, 2 Dec 2015 15:29:31 +0800 Subject: [PATCH 13/18] Update jsdoc comment function return description --- lib/models/mongo.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/models/mongo.js b/lib/models/mongo.js index 2d3f636..5233ccc 100644 --- a/lib/models/mongo.js +++ b/lib/models/mongo.js @@ -122,8 +122,10 @@ Mongo.prototype.setUserMapping = function (elasticUrl, userId, instanceShortHash }; /** - * Find nested directUrl object on a naviEntry document that is a masterPod branch + * Find nested directUrl object and its key (shortHash on a naviEntry document that is a masterPod + * branch * @param {String} branchName + * @return {Object} */ Mongo.findMasterPodBranch = function (naviEntry) { var logData = { From 59154edbe5dd2f07ac2d8a98b64f21583fb7fc38 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Wed, 2 Dec 2015 15:30:20 +0800 Subject: [PATCH 14/18] Fix minor whitespace --- lib/models/proxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index f1d6e8a..9e5b1e9 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -43,7 +43,7 @@ function Proxy () { req: req }; var errorUrl = errorPage.generateErrorUrl('unresponsive', { - shortHash: req.shortHash, + shortHash: req.shortHash, elasticUrl: req.elasticUrl }); var targetHost = getHostAndAugmentReq(req, errorUrl); From 228f9f99ffef85ce71972fd8b64a629a546e546c Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Wed, 2 Dec 2015 15:35:07 +0800 Subject: [PATCH 15/18] Revert "Update trace log metadata" This reverts commit 689977ab00bb95ea2991c182c3b9255d3613ec27. Conflicts: lib/models/proxy.js --- lib/models/proxy.js | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index 9e5b1e9..cc12504 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -42,16 +42,27 @@ function Proxy () { err: err, req: req }; - var errorUrl = errorPage.generateErrorUrl('unresponsive', { - shortHash: req.shortHash, - elasticUrl: req.elasticUrl - }); - var targetHost = getHostAndAugmentReq(req, errorUrl); - log.trace(put({ - targetHost: targetHost, - errorUrl: errorUrl - }, logData), 'proxy.on error'); - self.proxy.web(req, res, { target: targetHost }); + log.error(logData, 'proxy.on error'); + /** + * Question, Anand: + * I am not sure what the original intended purpose of this logic was. I think I should delete + * the req.didError conditional check and set. + */ + if (!req.didError) { + req.didError = true; + var errorUrl = errorPage.generateErrorUrl('unresponsive', { + shortHash: req.shortHash, + elasticUrl: req.elasticUrl + }); + var targetHost = getHostAndAugmentReq(req, errorUrl); + log.trace(put({ + targetHost: targetHost, + errorUrl: errorUrl + }, logData), 'proxy.on error !req.didError'); + self.proxy.web(req, res, { target: targetHost }); + } else { + log.trace(logData, 'proxy.on error req.didError'); + } }); /** From 82e600db74d0fa162073b9a6554cd669f24d8c71 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Wed, 2 Dec 2015 15:36:23 +0800 Subject: [PATCH 16/18] Add descriptive comment of proxy edgecase logic --- lib/models/proxy.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/models/proxy.js b/lib/models/proxy.js index cc12504..1aa77f0 100644 --- a/lib/models/proxy.js +++ b/lib/models/proxy.js @@ -44,9 +44,9 @@ function Proxy () { }; log.error(logData, 'proxy.on error'); /** - * Question, Anand: - * I am not sure what the original intended purpose of this logic was. I think I should delete - * the req.didError conditional check and set. + * we need to keep the didError logic. sometimes (not sure why) but we try to proxy the same + * request twice (in some network cases) and navi will crash saying you can't send on a request + * that is already ended */ if (!req.didError) { req.didError = true; From 1ad7d698e581d40604d99fbdbaa3fb17925b4832 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Wed, 2 Dec 2015 15:42:37 +0800 Subject: [PATCH 17/18] Add unit test for api._processTargetInstance --- test/unit/api.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/unit/api.js b/test/unit/api.js index d08181c..6159089 100644 --- a/test/unit/api.js +++ b/test/unit/api.js @@ -219,6 +219,16 @@ describe('api.js unit test', function () { }); describe('_processTargetInstance', function () { + beforeEach(function (done) { + sinon.stub(api, '_getDestinationProxyUrl'); + done(); + }); + + afterEach(function (done) { + api._getDestinationProxyUrl.restore(); + done(); + }); + it('should next with error if !targetNaviEntryInstance', function (done) { api._processTargetInstance(null, '', '', {}, function (err) { expect(err.message).to.equal('Not Found'); @@ -226,7 +236,7 @@ describe('api.js unit test', function () { }); }); - it('should set req.targetHost if !running', function (done) { + it('should set req.targetHost to error url if !running', function (done) { var req = {}; var reqUrl = 'api-staging-codenow.runnableapp.com'; api._processTargetInstance({ @@ -239,6 +249,20 @@ describe('api.js unit test', function () { done(); }); }); + + it('should set req.targetHost to container host & port if running', function (done) { + api._getDestinationProxyUrl.returns('http://0.0.0.0:600'); + var req = {}; + var reqUrl = 'api-staging-codenow.runnableapp.com'; + api._processTargetInstance({ + running: true, + branch: 'master' + }, '55555', reqUrl, req, function (err) { + expect(err).to.be.undefined(); + expect(req.targetHost).to.equal('http://0.0.0.0:600'); + done(); + }); + }); }); describe('api.getTargetHost', function () { From f604255fa79b8aaf31db86b2f5548f9b9d8a2722 Mon Sep 17 00:00:00 2001 From: Casey Flynn Date: Thu, 3 Dec 2015 16:44:23 +0800 Subject: [PATCH 18/18] Remove code comments --- test/unit/api.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/unit/api.js b/test/unit/api.js index 14efbf1..4c6d08c 100644 --- a/test/unit/api.js +++ b/test/unit/api.js @@ -517,8 +517,6 @@ describe('api.js unit test', function () { it('should default to masterPod instance if no associations/dns-mappings defined', function (done) { - log.info('===========================') - sinon.stub(mongo.constructor, 'findMasterPodBranch'); mongo.constructor.findMasterPodBranch.onFirstCall().returns({}); mongo.constructor.findMasterPodBranch.onSecondCall().returns(undefined); @@ -529,14 +527,11 @@ describe('api.js unit test', function () { }); api.getTargetHost(req, {}, function () { - - log.info('===========================') sinon.assert.calledWith(api._processTargetInstance, sinon.match({ masterPod: true })); sinon.assert.calledWith(api._processTargetInstance) mongo.constructor.findMasterPodBranch.restore(); - done(); }); });