Merge pull request #1864 from overleaf/ta-analytics-backoff-strategy

Update Analytics Backoff Strategy

GitOrigin-RevId: 9a3dc11ee19ff03432730a36617208ac7f58c5be

services/web/app/src/Features/Analytics/AnalyticsController.js

@@ -1,39 +1,21 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let AnalyticsController
 const AnalyticsManager = require('./AnalyticsManager')
 const Errors = require('../Errors/Errors')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const InstitutionsAPI = require('../Institutions/InstitutionsAPI')
 const GeoIpLookup = require('../../infrastructure/GeoIpLookup')
 
-module.exports = AnalyticsController = {
+module.exports = {
   updateEditingSession(req, res, next) {
     const userId = AuthenticationController.getLoggedInUserId(req)
     const { projectId } = req.params
     let countryCode = null
 
-    if (userId != null) {
-      return GeoIpLookup.getDetails(req.ip, function(err, geoDetails) {
-        if (
-          (geoDetails != null ? geoDetails.country_code : undefined) != null &&
-          geoDetails.country_code !== ''
-        ) {
+    if (userId) {
+      GeoIpLookup.getDetails(req.ip, function(err, geoDetails) {
+        if (!err && geoDetails && geoDetails.country_code) {
           countryCode = geoDetails.country_code
         }
-        return AnalyticsManager.updateEditingSession(
+        AnalyticsManager.updateEditingSession(
           userId,
           projectId,
           countryCode,
@@ -41,34 +23,29 @@ module.exports = AnalyticsController = {
         )
       })
     } else {
-      return res.send(204)
+      res.send(204)
     }
   },
 
   recordEvent(req, res, next) {
-    const user_id =
+    const userId =
       AuthenticationController.getLoggedInUserId(req) || req.sessionID
-    return AnalyticsManager.recordEvent(
-      user_id,
-      req.params.event,
-      req.body,
-      error => respondWith(error, res, next)
+    AnalyticsManager.recordEvent(userId, req.params.event, req.body, error =>
+      respondWith(error, res, next)
     )
   },
 
   licences(req, res, next) {
-    const { resource_id, start_date, end_date, lag } = req.query
-    return InstitutionsAPI.getInstitutionLicences(
-      resource_id,
-      start_date,
-      end_date,
-      lag,
+    InstitutionsAPI.getInstitutionLicences(
+      req.query.resource_id,
+      req.query.start_date,
+      req.query.end_date,
+      req.query.lag,
       function(error, licences) {
-        if (error != null) {
+        if (error) {
           return next(error)
-        } else {
-          return res.send(licences)
         }
+        res.send(licences)
       }
     )
   }
@@ -77,10 +54,10 @@ module.exports = AnalyticsController = {
 var respondWith = function(error, res, next) {
   if (error instanceof Errors.ServiceNotConfiguredError) {
     // ignore, no-op
-    return res.send(204)
-  } else if (error != null) {
-    return next(error)
+    res.send(204)
+  } else if (error) {
+    next(error)
   } else {
-    return res.send(204)
+    res.send(204)
   }
 }

services/web/app/src/Features/Analytics/AnalyticsManager.js

@@ -1,119 +1,106 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
-const _ = require('underscore')
-const request = require('requestretry')
+const request = require('request')
+const FaultTolerantRequest = require('../../infrastructure/FaultTolerantRequest')
 const Errors = require('../Errors/Errors')
 
-const isProduction =
-  (process.env['NODE_ENV'] || '').toLowerCase() === 'production'
-const isTest = process.env['MOCHA_GREP'] !== undefined
-
-const makeFaultTolerantRequest = function(userId, options, callback) {
+// check that the request should be made: ignore smoke test user and ensure the
+// analytics service is configured
+const checkAnalyticsRequest = function(userId) {
   if (
-    userId + '' ===
-    (settings.smokeTest != null ? settings.smokeTest.userId : undefined) + ''
+    settings.smokeTest &&
+    settings.smokeTest.userId &&
+    settings.smokeTest.userId.toString() === userId.toString()
   ) {
-    return callback()
+    // ignore smoke test user
+    return { error: null, skip: true }
   }
 
-  options = Object.assign(options, {
-    delayStrategy: exponentialBackoffStrategy(),
-    timeout: 30000
-  })
+  if (!settings.apis.analytics) {
+    return {
+      error: new Errors.ServiceNotConfiguredError(
+        'Analytics service not configured'
+      ),
+      skip: true
+    }
+  }
+
+  return { error: null, skip: false }
+}
 
+// prepare the request: set `fromv2` param and full URL
+const prepareAnalyticsRequest = function(options) {
   if (settings.overleaf != null) {
     options.qs = Object.assign({}, options.qs, { fromV2: 1 })
   }
 
-  makeRequest(options, function(err) {
-    if (err != null) {
-      return logger.err({ err }, 'Request to analytics failed')
-    }
-  })
+  const urlPath = options.url
+  options.url = `${settings.apis.analytics.url}${urlPath}`
+
+  options.timeout = options.timeout || 30000
 
-  return callback() // Do not wait for all the attempts
+  return options
 }
 
-var makeRequest = function(opts, callback) {
-  if (
-    __guard__(
-      settings.apis != null ? settings.apis.analytics : undefined,
-      x => x.url
-    ) != null
-  ) {
-    const urlPath = opts.url
-    opts.url = `${settings.apis.analytics.url}${urlPath}`
-    return request(opts, callback)
-  } else {
-    return callback(
-      new Errors.ServiceNotConfiguredError('Analytics service not configured')
-    )
+// make the request to analytics after checking and preparing it.
+// request happens asynchronously in the background and will be retried on error
+const makeAnalyticsBackgroundRequest = function(userId, options, callback) {
+  let { error, skip } = checkAnalyticsRequest(userId)
+  if (error || skip) {
+    return callback(error)
   }
-}
+  prepareAnalyticsRequest(options)
 
-// Set an exponential backoff to retry calls to analytics. First retry will
-// happen after 4s, then 8, 16, 32, 64...
-var exponentialBackoffStrategy = function() {
-  let attempts = 1 // This won't be called until there has been 1 failure
+  // With the tweaked parameter values (BACKOFF_BASE=3000, BACKOFF_MULTIPLIER=3):
+  // - the 6th attempt (maxAttempts=6) will run after 5.5 to 11.5 minutes
+  // - the 9th attempt (maxAttempts=9) will run after 86 to 250 minutes
+  options.maxAttempts = options.maxAttempts || 9
+  options.backoffBase = options.backoffBase || 3000
+  options.backoffMultiplier = options.backoffMultiplier || 3
 
-  return function() {
-    attempts += 1
-    return exponentialBackoffDelay(attempts)
-  }
+  FaultTolerantRequest.backgroundRequest(options, callback)
 }
 
-var exponentialBackoffDelay = function(attempts) {
-  const delay = Math.pow(2, attempts) * 1000
-
-  if (isProduction && !isTest) {
-    logger.warn(
-      'Error comunicating with the analytics service. ' +
-        `Will try again attempt ${attempts} in ${delay}ms`
-    )
+// make synchronous request to analytics without retries after checking and
+// preparing it.
+const makeAnalyticsRequest = function(userId, options, callback) {
+  let { error, skip } = checkAnalyticsRequest(userId)
+  if (error || skip) {
+    return callback(error)
   }
+  prepareAnalyticsRequest(options)
 
-  return delay
+  request(options, callback)
 }
 
 module.exports = {
-  identifyUser(user_id, old_user_id, callback) {
-    if (callback == null) {
-      callback = function(error) {}
+  identifyUser(userId, oldUserId, callback) {
+    if (!callback) {
+      // callback is optional
+      callback = () => {}
     }
+
     const opts = {
       body: {
-        old_user_id
+        old_user_id: oldUserId
       },
       json: true,
       method: 'POST',
-      timeout: 1000,
-      url: `/user/${user_id}/identify`
+      url: `/user/${userId}/identify`
     }
-    return makeRequest(opts, callback)
+    makeAnalyticsBackgroundRequest(userId, opts, callback)
   },
 
-  recordEvent(user_id, event, segmentation, callback) {
+  recordEvent(userId, event, segmentation, callback) {
     if (segmentation == null) {
+      // segmentation is optional
       segmentation = {}
     }
-    if (callback == null) {
-      callback = function(error) {}
+    if (!callback) {
+      // callback is optional
+      callback = () => {}
     }
+
     const opts = {
       body: {
         event,
@@ -121,17 +108,18 @@ module.exports = {
       },
       json: true,
       method: 'POST',
-      url: `/user/${user_id}/event`,
-      maxAttempts: 7 // Give up after ~ 8min
+      url: `/user/${userId}/event`
     }
 
-    return makeFaultTolerantRequest(user_id, opts, callback)
+    makeAnalyticsBackgroundRequest(userId, opts, callback)
   },
 
   updateEditingSession(userId, projectId, countryCode, callback) {
-    if (callback == null) {
-      callback = function(error) {}
+    if (!callback) {
+      // callback is optional
+      callback = () => {}
     }
+
     const query = {
       userId,
       projectId
@@ -145,16 +133,14 @@ module.exports = {
       method: 'PUT',
       url: '/editingSession',
       qs: query,
-      maxAttempts: 6 // Give up after ~ 4min
+      maxAttempts: 6 // dont retry for too long as session ping timestamp are
+      // recorded when the request is received on the analytics
     }
 
-    return makeFaultTolerantRequest(userId, opts, callback)
+    makeAnalyticsBackgroundRequest(userId, opts, callback)
   },
 
-  getLastOccurrence(user_id, event, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
+  getLastOccurrence(userId, event, callback) {
     const opts = {
       body: {
         event
@@ -162,22 +148,16 @@ module.exports = {
       json: true,
       method: 'POST',
       timeout: 1000,
-      url: `/user/${user_id}/event/last_occurrence`
+      url: `/user/${userId}/event/last_occurrence`
     }
-    return makeRequest(opts, function(err, response, body) {
+    makeAnalyticsRequest(userId, opts, function(err, response, body) {
       if (err != null) {
         console.log(response, opts)
-        logger.err({ user_id, err }, 'error getting last occurance of event')
-        return callback(err)
+        logger.err({ userId, err }, 'error getting last occurance of event')
+        callback(err)
       } else {
-        return callback(null, body)
+        callback(null, body)
       }
     })
   }
 }
-
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
-}

services/web/app/src/Features/Analytics/AnalyticsProxy.js

@@ -1,15 +1,3 @@
-/* eslint-disable
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const settings = require('settings-sharelatex')
 const Errors = require('../Errors/Errors')
 const httpProxy = require('express-http-proxy')
@@ -17,25 +5,7 @@ const URL = require('url')
 
 module.exports = {
   call(basePath) {
-    const analyticsUrl = __guard__(
-      __guard__(
-        settings != null ? settings.apis : undefined,
-        x1 => x1.analytics
-      ),
-      x => x.url
-    )
-    if (analyticsUrl != null) {
-      return httpProxy(analyticsUrl, {
-        proxyReqPathResolver(req) {
-          const requestPath = URL.parse(req.url).path
-          return `${basePath}${requestPath}`
-        },
-        proxyReqOptDecorator(proxyReqOpts, srcReq) {
-          proxyReqOpts.headers = {} // unset all headers
-          return proxyReqOpts
-        }
-      })
-    } else {
+    if (!settings.apis.analytics) {
       return (req, res, next) =>
         next(
           new Errors.ServiceNotConfiguredError(
@@ -43,11 +13,16 @@ module.exports = {
           )
         )
     }
-  }
-}
 
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
+    return httpProxy(settings.apis.analytics.url, {
+      proxyReqPathResolver(req) {
+        const requestPath = URL.parse(req.url).path
+        return `${basePath}${requestPath}`
+      },
+      proxyReqOptDecorator(proxyReqOpts, srcReq) {
+        proxyReqOpts.headers = {} // unset all headers
+        return proxyReqOpts
+      }
+    })
+  }
 }

services/web/app/src/Features/Analytics/AnalyticsRouter.js

@@ -1,10 +1,3 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Sanity-check the conversion and remove this comment.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const AuthenticationController = require('./../Authentication/AuthenticationController')
 const AnalyticsController = require('./AnalyticsController')
 const AnalyticsProxy = require('./AnalyticsProxy')
@@ -36,7 +29,7 @@ module.exports = {
       AnalyticsProxy.call('/recentV1TemplateIdsActivity')
     )
 
-    return publicApiRouter.use(
+    publicApiRouter.use(
       '/analytics/uniExternalCollaboration',
       AuthenticationController.httpAuth,
       AnalyticsProxy.call('/uniExternalCollaboration')

services/web/app/src/infrastructure/FaultTolerantRequest.js

@@ -0,0 +1,85 @@
+const logger = require('logger-sharelatex')
+const request = require('requestretry')
+
+const isProduction =
+  (process.env['NODE_ENV'] || '').toLowerCase() === 'production'
+const isTest = process.env['MOCHA_GREP'] !== undefined
+
+const BACKOFF_MAX_TRIES = 3
+const BACKOFF_BASE = 500
+const BACKOFF_MULTIPLIER = 1.5
+const BACKOFF_RANDOM_FACTOR = 0.5
+
+//
+// Use an exponential backoff to retry requests
+//
+// This is based on what the Google HTTP client does:
+// https://developers.google.com/api-client-library/java/google-http-java-client/reference/1.20.0/com/google/api/client/util/ExponentialBackOff
+//
+let FaultTolerantRequest
+module.exports = FaultTolerantRequest = {
+  request: function(options, callback) {
+    options = Object.assign(
+      {
+        maxAttempts: BACKOFF_MAX_TRIES,
+        backoffBase: BACKOFF_BASE,
+        backoffMultiplier: BACKOFF_MULTIPLIER,
+        backoffRandomFactor: BACKOFF_RANDOM_FACTOR
+      },
+      options
+    )
+
+    options.delayStrategy = FaultTolerantRequest.exponentialDelayStrategy(
+      options.backoffBase,
+      options.backoffMultiplier,
+      options.backoffRandomFactor
+    )
+
+    request(options, callback)
+  },
+
+  backgroundRequest: function(options, callback) {
+    FaultTolerantRequest.request(options, function(err) {
+      if (err) {
+        return logger.err(
+          { err, url: options.url, query: options.qs, body: options.body },
+          'Background request failed'
+        )
+      }
+    })
+
+    callback() // Do not wait for all the attempts
+  },
+
+  exponentialDelayStrategy: function(
+    backoffBase,
+    backoffMultiplier,
+    backoffRandomFactor
+  ) {
+    let backoff = backoffBase
+
+    return function() {
+      const delay = exponentialDelay(backoff, backoffRandomFactor)
+      backoff *= backoffMultiplier
+      return delay
+    }
+  }
+}
+
+function exponentialDelay(backoff, backoffRandomFactor) {
+  // set delay to `backoff` initially
+  let delay = backoff
+
+  // adds randomness
+  delay *= 1 - backoffRandomFactor + 2 * Math.random() * backoffRandomFactor
+
+  // round value as it's already in milliseconds
+  delay = Math.round(delay)
+
+  // log retries in production
+  if (isProduction && !isTest) {
+    logger.warn(`Background request failed. Will try again in ${delay}ms`)
+  }
+
+  return delay
+}

services/web/test/unit/src/Analytics/AnalyticsControllerTests.js

@@ -1,25 +1,10 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const should = require('chai').should()
 const SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
 const path = require('path')
 const modulePath = path.join(
   __dirname,
   '../../../../app/src/Features/Analytics/AnalyticsController'
 )
 const sinon = require('sinon')
-const { expect } = require('chai')
 
 describe('AnalyticsController', function() {
   beforeEach(function() {
@@ -49,7 +34,7 @@ describe('AnalyticsController', function() {
       }
     })
 
-    return (this.res = { send() {} })
+    this.res = { send() {} }
   })
 
   describe('updateEditingSession', function() {
@@ -59,32 +44,32 @@ describe('AnalyticsController', function() {
           projectId: 'a project id'
         }
       }
-      return (this.GeoIpLookup.getDetails = sinon
+      this.GeoIpLookup.getDetails = sinon
         .stub()
-        .callsArgWith(1, null, { country_code: 'XY' }))
+        .callsArgWith(1, null, { country_code: 'XY' })
     })
 
-    return it('delegates to the AnalyticsManager', function(done) {
+    it('delegates to the AnalyticsManager', function(done) {
       this.AuthenticationController.getLoggedInUserId.returns('1234')
       this.controller.updateEditingSession(this.req, this.res)
 
       this.AnalyticsManager.updateEditingSession
         .calledWith('1234', 'a project id', 'XY')
         .should.equal(true)
-      return done()
+      done()
     })
   })
 
   describe('recordEvent', function() {
     beforeEach(function() {
-      return (this.req = {
+      this.req = {
         params: {
           event: 'i_did_something'
         },
         body: 'stuff',
         sessionID: 'sessionIDHere',
         session: {}
-      })
+      }
     })
 
     it('should use the user_id', function(done) {
@@ -93,21 +78,21 @@ describe('AnalyticsController', function() {
       this.AnalyticsManager.recordEvent
         .calledWith('1234', this.req.params['event'], this.req.body)
         .should.equal(true)
-      return done()
+      done()
     })
 
-    return it('should use the session id', function(done) {
+    it('should use the session id', function(done) {
       this.controller.recordEvent(this.req, this.res)
       this.AnalyticsManager.recordEvent
         .calledWith(this.req.sessionID, this.req.params['event'], this.req.body)
         .should.equal(true)
-      return done()
+      done()
     })
   })
 
-  return describe('licences', function() {
+  describe('licences', function() {
     beforeEach(function() {
-      return (this.req = {
+      this.req = {
         query: {
           resource_id: 1,
           start_date: '1514764800',
@@ -116,10 +101,10 @@ describe('AnalyticsController', function() {
         },
         sessionID: 'sessionIDHere',
         session: {}
-      })
+      }
     })
 
-    return it('should trigger institutions api to fetch licences graph data', function(done) {
+    it('should trigger institutions api to fetch licences graph data', function(done) {
       this.controller.licences(this.req, this.res)
       this.InstitutionsAPI.getInstitutionLicences
         .calledWith(
@@ -129,7 +114,7 @@ describe('AnalyticsController', function() {
           this.req.query['lag']
         )
         .should.equal(true)
-      return done()
+      done()
     })
   })
 })

services/web/test/unit/src/Analytics/AnalyticsManagerTests.js

@@ -0,0 +1,115 @@
+const SandboxedModule = require('sandboxed-module')
+const path = require('path')
+const modulePath = path.join(
+  __dirname,
+  '../../../../app/src/Features/Analytics/AnalyticsManager'
+)
+const sinon = require('sinon')
+const { expect } = require('chai')
+const Errors = require('../../../../app/src/Features/Errors/Errors')
+
+describe('AnalyticsManager', function() {
+  beforeEach(function() {
+    this.fakeUserId = '123abc'
+    this.settings = {
+      overleaf: true,
+      apis: { analytics: { url: 'analytics.test' } }
+    }
+    this.backgroundRequest = sinon.stub().yields()
+    this.request = sinon.stub().yields()
+    this.AnalyticsManager = SandboxedModule.require(modulePath, {
+      requires: {
+        'settings-sharelatex': this.settings,
+        '../../infrastructure/FaultTolerantRequest': {
+          backgroundRequest: this.backgroundRequest
+        },
+        '../Errors/Errors': Errors,
+        request: this.request,
+        'logger-sharelatex': {
+          log() {}
+        }
+      }
+    })
+  })
+
+  describe('checkAnalyticsRequest', function() {
+    it('ignores smoke test user', function(done) {
+      this.settings.smokeTest = { userId: this.fakeUserId }
+      this.AnalyticsManager.identifyUser(this.fakeUserId, '', error => {
+        expect(error).to.not.exist
+        sinon.assert.notCalled(this.request)
+        done()
+      })
+    })
+
+    it('return error if analytics service is not configured', function(done) {
+      this.settings.apis.analytics = null
+      this.AnalyticsManager.identifyUser(this.fakeUserId, '', error => {
+        expect(error).to.be.instanceof(Errors.ServiceNotConfiguredError)
+        sinon.assert.notCalled(this.request)
+        done()
+      })
+    })
+  })
+
+  describe('makes correct request to analytics', function() {
+    it('identifyUser', function(done) {
+      const oldUserId = '456def'
+      this.AnalyticsManager.identifyUser(this.fakeUserId, oldUserId, error => {
+        expect(error).to.not.exist
+        sinon.assert.calledWithMatch(this.backgroundRequest, {
+          body: { old_user_id: oldUserId },
+          url: 'analytics.test/user/123abc/identify'
+        })
+        done()
+      })
+    })
+
+    it('recordEvent', function(done) {
+      const event = 'fake-event'
+      this.AnalyticsManager.recordEvent(this.fakeUserId, event, null, error => {
+        expect(error).to.not.exist
+        sinon.assert.calledWithMatch(this.backgroundRequest, {
+          body: { event },
+          qs: { fromV2: 1 },
+          url: 'analytics.test/user/123abc/event',
+          timeout: 30000,
+          maxAttempts: 9,
+          backoffBase: 3000,
+          backoffMultiplier: 3
+        })
+        done()
+      })
+    })
+
+    it('updateEditingSession', function(done) {
+      const projectId = '789ghi'
+      const countryCode = 'fr'
+      this.AnalyticsManager.updateEditingSession(
+        this.fakeUserId,
+        projectId,
+        countryCode,
+        error => {
+          expect(error).to.not.exist
+          sinon.assert.calledWithMatch(this.backgroundRequest, {
+            qs: { userId: this.fakeUserId, projectId, countryCode, fromV2: 1 },
+            url: 'analytics.test/editingSession'
+          })
+          done()
+        }
+      )
+    })
+
+    it('getLastOccurrence', function(done) {
+      const event = 'fake-event'
+      this.AnalyticsManager.getLastOccurrence(this.fakeUserId, event, error => {
+        expect(error).to.not.exist
+        sinon.assert.calledWithMatch(this.request, {
+          body: { event },
+          url: 'analytics.test/user/123abc/event/last_occurrence'
+        })
+        done()
+      })
+    })
+  })
+})

services/web/test/unit/src/infrastructure/FaultTolerantRequestTests.js

@@ -0,0 +1,118 @@
+const SandboxedModule = require('sandboxed-module')
+const path = require('path')
+const modulePath = path.join(
+  __dirname,
+  '../../../../app/src/infrastructure/FaultTolerantRequest'
+)
+const sinon = require('sinon')
+const { expect } = require('chai')
+
+describe('FaultTolerantRequest', function() {
+  beforeEach(function() {
+    this.request = sinon.stub().yields()
+    this.logger = {
+      err: sinon.stub()
+    }
+    this.FaultTolerantRequest = SandboxedModule.require(modulePath, {
+      requires: {
+        requestretry: this.request,
+        'logger-sharelatex': this.logger
+      }
+    })
+  })
+
+  describe('exponentialBackoffStrategy', function() {
+    it('returns delays within expected range with default options', function() {
+      this.FaultTolerantRequest.request({}, () => {})
+      const delayStrategy = this.request.lastCall.args[0].delayStrategy
+      expect(delayStrategy()).to.be.closeTo(500, 250) // attempt 1
+      expect(delayStrategy()).to.be.closeTo(750, 375) // attempt 2
+      expect(delayStrategy()).to.be.closeTo(1125, 563) // attempt 3
+      expect(delayStrategy()).to.be.closeTo(1688, 844) // attempt 4
+      expect(delayStrategy()).to.be.closeTo(2531, 1266) // attempt 5
+      expect(delayStrategy()).to.be.closeTo(3797, 1899) // attempt 6
+      expect(delayStrategy()).to.be.closeTo(5695, 2848) // attempt 7
+      expect(delayStrategy()).to.be.closeTo(8543, 4272) // attempt 8
+      expect(delayStrategy()).to.be.closeTo(12814, 6408) // attempt 9
+      expect(delayStrategy()).to.be.closeTo(19222, 9610) // attempt 10
+    })
+
+    it('returns delays within expected range with custom options', function() {
+      const delayStrategy = this.FaultTolerantRequest.exponentialDelayStrategy(
+        3000,
+        3,
+        0.5
+      )
+      expect(delayStrategy()).to.be.closeTo(3000, 1500) // attempt 1
+      expect(delayStrategy()).to.be.closeTo(9000, 4500) // attempt 2
+      expect(delayStrategy()).to.be.closeTo(27000, 13500) // attempt 3
+      expect(delayStrategy()).to.be.closeTo(81000, 40500) // attempt 4
+      expect(delayStrategy()).to.be.closeTo(243000, 121500) // attempt 5
+      expect(delayStrategy()).to.be.closeTo(729000, 364500) // attempt 6
+      expect(delayStrategy()).to.be.closeTo(2187000, 1093500) // attempt 7
+      expect(delayStrategy()).to.be.closeTo(6561000, 3280500) // attempt 8
+      expect(delayStrategy()).to.be.closeTo(19683000, 9841500) // attempt 9
+      expect(delayStrategy()).to.be.closeTo(59049000, 29524500) // attempt 10
+    })
+  })
+
+  describe('request', function() {
+    it('sets retry options', function(done) {
+      this.FaultTolerantRequest.request({}, error => {
+        expect(error).to.not.exist
+        sinon.assert.calledOnce(this.request)
+
+        const { delayStrategy, maxAttempts } = this.request.lastCall.args[0]
+        expect(delayStrategy).to.be.a('function')
+        expect(maxAttempts).to.be.a('number')
+        done()
+      })
+    })
+
+    it("don't overwrite retry options", function(done) {
+      const customMaxAttempts = Math.random()
+      const customBase = Math.random()
+      const customMultiplier = Math.random()
+      const customRandomFactor = Math.random()
+      this.FaultTolerantRequest.request(
+        {
+          maxAttempts: customMaxAttempts,
+          backoffBase: customBase,
+          backoffMultiplier: customMultiplier,
+          backoffRandomFactor: customRandomFactor
+        },
+        error => {
+          expect(error).to.not.exist
+
+          const {
+            maxAttempts,
+            backoffBase,
+            backoffMultiplier,
+            backoffRandomFactor
+          } = this.request.lastCall.args[0]
+          expect(maxAttempts).to.equal(customMaxAttempts)
+          expect(backoffBase).to.equal(customBase)
+          expect(backoffMultiplier).to.equal(customMultiplier)
+          expect(backoffRandomFactor).to.equal(customRandomFactor)
+          done()
+        }
+      )
+    })
+  })
+
+  describe('backgroundRequest', function() {
+    it('logs error in the background', function(done) {
+      this.request.yields(new Error('Nope'))
+      this.logger.err = (options, message) => {
+        expect(options.url).to.equal('test.url')
+        done()
+      }
+      this.FaultTolerantRequest.backgroundRequest(
+        { url: 'test.url' },
+        error => {
+          expect(error).to.not.exist
+        }
+      )
+    })
+  })
+})