From 1c15e0966cfb03927a2c9bc5b5ce921bd0eff653 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 8 Feb 2018 11:42:48 +0000 Subject: [PATCH 1/3] Add a retry to analytics requests --- .../Features/Analytics/AnalyticsManager.coffee | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 007701f3a9..a1056481d6 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -6,10 +6,24 @@ Errors = require '../Errors/Errors' makeRequest = (opts, callback)-> + retryTimings = [1, 2, 4, 8, 16, 32, 32, 32] if settings.apis?.analytics?.url? urlPath = opts.url opts.url = "#{settings.apis.analytics.url}#{urlPath}" - request opts, callback + iteration = 0 + _go = () -> + request opts, (err, response, data) -> + if err? + if iteration == retryTimings.length + logger.err {err, url: opts.url}, + "Error in analytics request, retries failed" + return callback(err) + backoffSeconds = retryTimings[iteration] + iteration += 1 + setTimeout(_go, backoffSeconds * 1000) + else + callback(null, response, data) + _go() else callback(new Errors.ServiceNotConfiguredError('Analytics service not configured')) From 811706167441cf5487020917e00bb083c7183177 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 8 Feb 2018 13:04:47 +0000 Subject: [PATCH 2/3] Make the retry-on-fail behaviour optional --- .../web/app/coffee/Features/Analytics/AnalyticsManager.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index a1056481d6..1cd187f1ac 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -14,7 +14,7 @@ makeRequest = (opts, callback)-> _go = () -> request opts, (err, response, data) -> if err? - if iteration == retryTimings.length + if iteration == retryTimings.length or !opts.retryOnFail logger.err {err, url: opts.url}, "Error in analytics request, retries failed" return callback(err) @@ -51,6 +51,7 @@ module.exports = method:"POST" timeout:1000 url: "/user/#{user_id}/event" + retryOnFail: true if settings.overleaf? opts.qs = {fromV2: 1} makeRequest opts, callback @@ -68,6 +69,7 @@ module.exports = qs: userId: userId projectId: projectId + retryOnFail: true if settings.overleaf? opts.qs.fromV2 = 1 makeRequest opts, callback From 0efc8136cb784dc2da011b242799ee7a0fab1a8c Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 12 Feb 2018 15:16:21 +0000 Subject: [PATCH 3/3] Use `requestretry` node package --- .../Analytics/AnalyticsManager.coffee | 24 +++++-------------- services/web/npm-shrinkwrap.json | 10 ++++++++ services/web/package.json | 1 + 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 1cd187f1ac..5a1266dc2d 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -1,29 +1,15 @@ settings = require "settings-sharelatex" logger = require "logger-sharelatex" _ = require "underscore" -request = require "request" +request = require "requestretry" Errors = require '../Errors/Errors' makeRequest = (opts, callback)-> - retryTimings = [1, 2, 4, 8, 16, 32, 32, 32] if settings.apis?.analytics?.url? urlPath = opts.url opts.url = "#{settings.apis.analytics.url}#{urlPath}" - iteration = 0 - _go = () -> - request opts, (err, response, data) -> - if err? - if iteration == retryTimings.length or !opts.retryOnFail - logger.err {err, url: opts.url}, - "Error in analytics request, retries failed" - return callback(err) - backoffSeconds = retryTimings[iteration] - iteration += 1 - setTimeout(_go, backoffSeconds * 1000) - else - callback(null, response, data) - _go() + request opts, callback else callback(new Errors.ServiceNotConfiguredError('Analytics service not configured')) @@ -51,7 +37,8 @@ module.exports = method:"POST" timeout:1000 url: "/user/#{user_id}/event" - retryOnFail: true + maxAttempts: 20 + retryDelay: 5000 if settings.overleaf? opts.qs = {fromV2: 1} makeRequest opts, callback @@ -69,7 +56,8 @@ module.exports = qs: userId: userId projectId: projectId - retryOnFail: true + maxAttempts: 20 + retryDelay: 5000 if settings.overleaf? opts.qs.fromV2 = 1 makeRequest opts, callback diff --git a/services/web/npm-shrinkwrap.json b/services/web/npm-shrinkwrap.json index 765fc1f03e..fc764510df 100644 --- a/services/web/npm-shrinkwrap.json +++ b/services/web/npm-shrinkwrap.json @@ -6926,6 +6926,11 @@ "from": "request@>=2.69.0 <3.0.0", "resolved": "https://registry.npmjs.org/request/-/request-2.83.0.tgz" }, + "requestretry": { + "version": "1.13.0", + "from": "requestretry@latest", + "resolved": "https://registry.npmjs.org/requestretry/-/requestretry-1.13.0.tgz" + }, "requests": { "version": "0.1.7", "from": "requests@>=0.1.7 <0.2.0", @@ -9309,6 +9314,11 @@ "resolved": "https://registry.npmjs.org/websocket-extensions/-/websocket-extensions-0.1.3.tgz", "dev": true }, + "when": { + "version": "3.7.8", + "from": "when@>=3.7.7 <4.0.0", + "resolved": "https://registry.npmjs.org/when/-/when-3.7.8.tgz" + }, "which": { "version": "1.0.9", "from": "which@>=1.0.5 <1.1.0", diff --git a/services/web/package.json b/services/web/package.json index 7cb82b05d6..78813ec260 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -71,6 +71,7 @@ "pug": "^2.0.0-beta6", "redis-sharelatex": "git+https://github.com/sharelatex/redis-sharelatex.git#v1.0.4", "request": "^2.69.0", + "requestretry": "^1.13.0", "requests": "^0.1.7", "rimraf": "2.2.6", "rolling-rate-limiter": "git+https://github.com/ShaneKilkelly/rolling-rate-limiter.git#master",