From c28e09076b5b5417e15e2605abb4821f19947864 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Nov 2014 13:33:03 +0000 Subject: [PATCH 1/6] Add missing logger include in smoke test --- services/web/test/smoke/coffee/SmokeTests.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/test/smoke/coffee/SmokeTests.coffee b/services/web/test/smoke/coffee/SmokeTests.coffee index 3fd47d9faf..c2fce1b2d0 100644 --- a/services/web/test/smoke/coffee/SmokeTests.coffee +++ b/services/web/test/smoke/coffee/SmokeTests.coffee @@ -8,6 +8,7 @@ Settings = require "settings-sharelatex" port = Settings.internal?.web?.port or Settings.port or 3000 cookeFilePath = "/tmp/smoke-test-cookie-#{port}.txt" buildUrl = (path) -> " -b #{cookeFilePath} --resolve 'smoke#{Settings.cookieDomain}:#{port}:127.0.0.1' http://smoke#{Settings.cookieDomain}:#{port}/#{path}?setLng=en" +logger = require "logger-sharelatex" describe "Opening", -> From d4af0fe36d83aac0ca537bc1b6cc13b44cfaac83 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Nov 2014 13:36:10 +0000 Subject: [PATCH 2/6] Wrap smoke test in domain to catch errors --- .../HealthCheck/HealthCheckController.coffee | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/HealthCheck/HealthCheckController.coffee b/services/web/app/coffee/Features/HealthCheck/HealthCheckController.coffee index 1924cc4338..1a82718299 100644 --- a/services/web/app/coffee/Features/HealthCheck/HealthCheckController.coffee +++ b/services/web/app/coffee/Features/HealthCheck/HealthCheckController.coffee @@ -3,23 +3,26 @@ Base = require("mocha/lib/reporters/base") redis = require("redis-sharelatex") settings = require("settings-sharelatex") redisCheck = redis.activeHealthCheckRedis(settings.redis.web) +logger = require "logger-sharelatex" +domain = require "domain" module.exports = HealthCheckController = check: (req, res, next = (error) ->) -> - mocha = new Mocha(reporter: Reporter(res), timeout: 10000) - mocha.addFile("test/smoke/js/SmokeTests.js") - mocha.run () -> - path = require.resolve(__dirname + "/../../../../test/smoke/js/SmokeTests.js") - delete require.cache[path] + d = domain.create() + d.on "error", (error) -> + logger.err err: error, "error in mocha" + d.run () -> + mocha = new Mocha(reporter: Reporter(res), timeout: 10000) + mocha.addFile("test/smoke/js/SmokeTests.js") + mocha.run () -> + path = require.resolve(__dirname + "/../../../../test/smoke/js/SmokeTests.js") + delete require.cache[path] checkRedis: (req, res, next)-> if redisCheck.isAlive() res.send 200 else res.send 500 - - - Reporter = (res) -> (runner) -> From 3578e41c9c4e8ba52e99501d8bdaa885375a92dc Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Nov 2014 13:39:07 +0000 Subject: [PATCH 3/6] Add null check into FileTypeManager isDirectory check --- .../web/app/coffee/Features/Uploads/FileTypeManager.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Uploads/FileTypeManager.coffee b/services/web/app/coffee/Features/Uploads/FileTypeManager.coffee index 44eed12db5..0c858af75a 100644 --- a/services/web/app/coffee/Features/Uploads/FileTypeManager.coffee +++ b/services/web/app/coffee/Features/Uploads/FileTypeManager.coffee @@ -24,7 +24,8 @@ module.exports = FileTypeManager = isDirectory: (path, callback = (error, result) ->) -> fs.stat path, (error, stats) -> - callback(error, stats.isDirectory()) + return callback(error) if error? + callback(null, stats?.isDirectory()) isBinary: (name, fsPath, callback = (error, result) ->) -> parts = name.split(".") From 970125b7a830a13fd695ed99bbdbe9554513a610 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Nov 2014 13:43:08 +0000 Subject: [PATCH 4/6] Check for null project in joinProject --- services/web/app/coffee/Features/Editor/EditorController.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index f8d3e5da37..4b19458e03 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -63,6 +63,7 @@ module.exports = EditorController = buildJoinProjectView: (project_id, user_id, callback = (error, project, privilegeLevel) ->) -> ProjectGetter.getProjectWithoutDocLines project_id, (error, project) -> return callback(error) if error? + return callback(new Error("not found")) if !project? ProjectGetter.populateProjectWithUsers project, (error, project) -> return callback(error) if error? UserGetter.getUser user_id, { isAdmin: true }, (error, user) -> From b8fdbdb40628b18ede02ac4f53aea3137597b485 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Nov 2014 13:58:41 +0000 Subject: [PATCH 5/6] Handle errors in request pipes --- services/web/app/coffee/Features/Blog/BlogController.coffee | 6 +++++- .../coffee/Features/StaticPages/UniversityController.coffee | 5 ++++- .../coffee/Features/Templates/TemplatesController.coffee | 5 ++++- services/web/app/coffee/infrastructure/OldAssetProxy.coffee | 5 ++++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Blog/BlogController.coffee b/services/web/app/coffee/Features/Blog/BlogController.coffee index 9d47a11224..54a716c789 100644 --- a/services/web/app/coffee/Features/Blog/BlogController.coffee +++ b/services/web/app/coffee/Features/Blog/BlogController.coffee @@ -22,6 +22,7 @@ module.exports = BlogController = logger.log url:url, "proxying request to blog api" request.get blogUrl, (err, r, data)-> + return next(err) if err? if r?.statusCode == 404 return ErrorController.notFound(req, res, next) data = data.trim() @@ -37,4 +38,7 @@ module.exports = BlogController = BlogController.getPage req, res _directProxy: (originUrl, res)-> - request.get(originUrl).pipe res \ No newline at end of file + upstream = request.get(originUrl) + upstream.on "error", (error) -> + logger.error err: error, "blog proxy error" + upstream.pipe res \ No newline at end of file diff --git a/services/web/app/coffee/Features/StaticPages/UniversityController.coffee b/services/web/app/coffee/Features/StaticPages/UniversityController.coffee index b5d848542e..d23fbfbeed 100644 --- a/services/web/app/coffee/Features/StaticPages/UniversityController.coffee +++ b/services/web/app/coffee/Features/StaticPages/UniversityController.coffee @@ -31,4 +31,7 @@ module.exports = UniversityController = UniversityController.getPage req, res _directProxy: (originUrl, res)-> - request.get(originUrl).pipe res \ No newline at end of file + upstream = request.get(originUrl) + upstream.on "error", (error) -> + logger.error err: error, "university proxy error" + upstream.pipe res \ No newline at end of file diff --git a/services/web/app/coffee/Features/Templates/TemplatesController.coffee b/services/web/app/coffee/Features/Templates/TemplatesController.coffee index 1354afdbe5..6e3ea5fe1e 100644 --- a/services/web/app/coffee/Features/Templates/TemplatesController.coffee +++ b/services/web/app/coffee/Features/Templates/TemplatesController.coffee @@ -27,7 +27,10 @@ module.exports = zipUrl = "#{settings.apis.web.url}#{zipUrl}" else zipUrl = "#{settings.apis.templates.url}#{zipUrl}" - request(zipUrl).pipe(writeStream) + zipReq = request(zipUrl) + zipReq.on "error", (error) -> + logger.error err: error, "error getting zip from template API" + zipReq.pipe(writeStream) writeStream.on 'close', -> ProjectUploadManager.createProjectFromZipArchive req.session.user._id, req.session.templateData.templateName, dumpPath, (err, project)-> if err? diff --git a/services/web/app/coffee/infrastructure/OldAssetProxy.coffee b/services/web/app/coffee/infrastructure/OldAssetProxy.coffee index 1407d46eca..7912f0ed14 100644 --- a/services/web/app/coffee/infrastructure/OldAssetProxy.coffee +++ b/services/web/app/coffee/infrastructure/OldAssetProxy.coffee @@ -8,6 +8,9 @@ module.exports = (req, res, next)-> redirectUrl = settings.proxyUrls[requestedUrl] if redirectUrl? logger.log redirectUrl:redirectUrl, reqUrl:req.url, "proxying url" - request(redirectUrl).pipe(res) + upstream = request(redirectUrl) + upstream.on "error", (error) -> + logger.error err: error, "error in OldAssetProxy" + upstream.pipe(res) else next() From afca9ba0cb8c9069b53abd444be1cef9fe86bd75 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Nov 2014 14:26:51 +0000 Subject: [PATCH 6/6] Fix unit tests --- .../coffee/Templates/TemplatesControllerTests.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/test/UnitTests/coffee/Templates/TemplatesControllerTests.coffee b/services/web/test/UnitTests/coffee/Templates/TemplatesControllerTests.coffee index c529342e54..8bf949c76a 100644 --- a/services/web/test/UnitTests/coffee/Templates/TemplatesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Templates/TemplatesControllerTests.coffee @@ -12,7 +12,10 @@ describe 'TemplatesController', -> beforeEach -> @request = sinon.stub() - @request.returns pipe:-> + @request.returns { + pipe:-> + on:-> + } @fs = { unlink : sinon.stub() createWriteStream : sinon.stub().returns(on:(_, cb)->cb())