From e886217b851541f9896a627b329fc2ca3cb337f8 Mon Sep 17 00:00:00 2001 From: Michael Walker Date: Fri, 2 Feb 2018 09:43:57 +0000 Subject: [PATCH 1/3] Bail out if history API request fails --- .../coffee/Features/History/HistoryController.coffee | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index f8f1578bd5..d158cd86c6 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -46,9 +46,14 @@ module.exports = HistoryController = "X-User-Id": user_id }, (error, response, body) -> return next(error) if error? - HistoryManager.injectUserDetails body, (error, data) -> - return next(error) if error? - res.json data + if 200 <= response.statusCode < 300 + HistoryManager.injectUserDetails body, (error, data) -> + return next(error) if error? + res.json data + else + error = new Error("history api responded with non-success code: #{response.statusCode}") + logger.error err: error, user_id: user_id, "error proxying request to history api" + next(error) buildHistoryServiceUrl: (useProjectHistory) -> # choose a history service, either document-level (trackchanges) From 217940d76f88cd86ef618a6170f09b01128dadbe Mon Sep 17 00:00:00 2001 From: Michael Walker Date: Fri, 2 Feb 2018 10:03:25 +0000 Subject: [PATCH 2/3] Fix failing unit tests --- .../web/test/unit/coffee/History/HistoryControllerTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee index 46bfa49f8f..06f2630b6b 100644 --- a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee +++ b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee @@ -122,7 +122,7 @@ describe "HistoryController", -> @res = json: sinon.stub() @next = sinon.stub() - @request.yields(null, {}, @data = "mock-data") + @request.yields(null, {statusCode: 200}, @data = "mock-data") @HistoryManager.injectUserDetails = sinon.stub().yields(null, @data_with_users = "mock-injected-data") describe "for a project with the project history flag", -> From 5987d6d79cbe9b000afb2ad5d2b6dceca3162fb8 Mon Sep 17 00:00:00 2001 From: Michael Walker Date: Fri, 2 Feb 2018 10:08:29 +0000 Subject: [PATCH 3/3] Add unit tests for failing history API --- .../History/HistoryControllerTests.coffee | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee index 06f2630b6b..7dc3c88169 100644 --- a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee +++ b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee @@ -182,3 +182,20 @@ describe "HistoryController", -> it "should return the data with users to the client", -> @res.json.calledWith(@data_with_users).should.equal true + + describe "proxyToHistoryApiAndInjectUserDetails (with the history API failing)", -> + beforeEach -> + @req = { url: "/mock/url", method: "POST", useProjectHistory: true } + @res = { json: sinon.stub() } + @next = sinon.stub() + @request.yields(null, {statusCode: 500}, @data = "mock-data") + @HistoryManager.injectUserDetails = sinon.stub().yields(null, @data_with_users = "mock-injected-data") + @HistoryController.proxyToHistoryApiAndInjectUserDetails @req, @res, @next + + it "should not inject the user data", -> + @HistoryManager.injectUserDetails + .calledWith(@data) + .should.equal false + + it "should not return the data with users to the client", -> + @res.json.calledWith(@data_with_users).should.equal false