Merge pull request #25200 from overleaf/revert-25023-ac-promisify-compile-controller

Revert "[web] Promisify ClsiCookieManager and CompileController"

GitOrigin-RevId: 190ee8d2be23687f092e762c5199a34bcdf37cf9

services/web/app/src/Features/Compile/ClsiCookieManager.js

@@ -1,15 +1,12 @@
 const { URL, URLSearchParams } = require('url')
 const OError = require('@overleaf/o-error')
 const Settings = require('@overleaf/settings')
-const {
-  fetchNothing,
-  fetchStringWithResponse,
-  RequestFailedError,
-} = require('@overleaf/fetch-utils')
+const request = require('request').defaults({ timeout: 30 * 1000 })
 const RedisWrapper = require('../../infrastructure/RedisWrapper')
 const Cookie = require('cookie')
 const logger = require('@overleaf/logger')
 const Metrics = require('@overleaf/metrics')
+const { promisifyAll } = require('@overleaf/promise-utils')
 
 const clsiCookiesEnabled = (Settings.clsiCookie?.key ?? '') !== ''
 
@@ -19,208 +16,235 @@ if (Settings.redis.clsi_cookie_secondary != null) {
   rclientSecondary = RedisWrapper.client('clsi_cookie_secondary')
 }
 
-const ClsiCookieManagerFactory = function (backendGroup) {
-  function buildKey(projectId, userId) {
-    if (backendGroup != null) {
-      return `clsiserver:${backendGroup}:${projectId}:${userId}`
-    } else {
-      return `clsiserver:${projectId}:${userId}`
-    }
-  }
+module.exports = function (backendGroup) {
+  const cookieManager = {
+    buildKey(projectId, userId) {
+      if (backendGroup != null) {
+        return `clsiserver:${backendGroup}:${projectId}:${userId}`
+      } else {
+        return `clsiserver:${projectId}:${userId}`
+      }
+    },
 
-  async function getServerId(
-    projectId,
-    userId,
-    compileGroup,
-    compileBackendClass
-  ) {
-    if (!clsiCookiesEnabled) {
-      return
-    }
-    const serverId = await rclient.get(buildKey(projectId, userId))
-
-    if (!serverId) {
-      return cookieManager.promises._populateServerIdViaRequest(
-        projectId,
-        userId,
-        compileGroup,
-        compileBackendClass
-      )
-    } else {
-      return serverId
-    }
-  }
-
-  async function _populateServerIdViaRequest(
-    projectId,
-    userId,
-    compileGroup,
-    compileBackendClass
-  ) {
-    const u = new URL(`${Settings.apis.clsi.url}/project/${projectId}/status`)
-    u.search = new URLSearchParams({
+    getServerId(
+      projectId,
+      userId,
       compileGroup,
       compileBackendClass,
-    }).toString()
-    let res
-    try {
-      res = await fetchNothing(u.href, {
-        method: 'POST',
-        signal: AbortSignal.timeout(30_000),
-      })
-    } catch (err) {
-      if (err instanceof RequestFailedError && err.response.status < 500) {
-        logger.warn(
-          { err, projectId },
-          'error requesting project status from clsi'
-        )
-        res = err.response
-      } else {
-        OError.tag(err, 'error getting initial server id for project', {
-          project_id: projectId,
-        })
-        throw err
+      callback
+    ) {
+      if (!clsiCookiesEnabled) {
+        return callback()
       }
-    }
+      rclient.get(this.buildKey(projectId, userId), (err, serverId) => {
+        if (err) {
+          return callback(err)
+        }
+        if (serverId == null || serverId === '') {
+          this._populateServerIdViaRequest(
+            projectId,
+            userId,
+            compileGroup,
+            compileBackendClass,
+            callback
+          )
+        } else {
+          callback(null, serverId)
+        }
+      })
+    },
 
-    if (!clsiCookiesEnabled) {
-      return
-    }
-    const serverId = cookieManager._parseServerIdFromResponse(res)
-    try {
-      await cookieManager.promises.setServerId(
+    _populateServerIdViaRequest(
+      projectId,
+      userId,
+      compileGroup,
+      compileBackendClass,
+      callback
+    ) {
+      const u = new URL(`${Settings.apis.clsi.url}/project/${projectId}/status`)
+      u.search = new URLSearchParams({
+        compileGroup,
+        compileBackendClass,
+      }).toString()
+      request.post(u.href, (err, res, body) => {
+        if (err) {
+          OError.tag(err, 'error getting initial server id for project', {
+            project_id: projectId,
+          })
+          return callback(err)
+        }
+        if (!clsiCookiesEnabled) {
+          return callback()
+        }
+        const serverId = this._parseServerIdFromResponse(res)
+        this.setServerId(
+          projectId,
+          userId,
+          compileGroup,
+          compileBackendClass,
+          serverId,
+          null,
+          function (err) {
+            if (err) {
+              logger.warn(
+                { err, projectId },
+                'error setting server id via populate request'
+              )
+            }
+            callback(err, serverId)
+          }
+        )
+      })
+    },
+
+    _parseServerIdFromResponse(response) {
+      const cookies = Cookie.parse(response.headers['set-cookie']?.[0] || '')
+      return cookies?.[Settings.clsiCookie.key]
+    },
+
+    checkIsLoadSheddingEvent(clsiserverid, compileGroup, compileBackendClass) {
+      request.get(
+        {
+          url: `${Settings.apis.clsi.url}/instance-state`,
+          qs: { clsiserverid, compileGroup, compileBackendClass },
+        },
+        (err, res, body) => {
+          if (err) {
+            Metrics.inc('clsi-lb-switch-backend', 1, {
+              status: 'error',
+            })
+            logger.warn({ err, clsiserverid }, 'cannot probe clsi VM')
+            return
+          }
+          const isStillRunning =
+            res.statusCode === 200 && body === `${clsiserverid},UP\n`
+          Metrics.inc('clsi-lb-switch-backend', 1, {
+            status: isStillRunning ? 'load-shedding' : 'cycle',
+          })
+        }
+      )
+    },
+
+    _getTTLInSeconds(clsiServerId) {
+      return (clsiServerId || '').includes('-reg-')
+        ? Settings.clsiCookie.ttlInSecondsRegular
+        : Settings.clsiCookie.ttlInSeconds
+    },
+
+    setServerId(
+      projectId,
+      userId,
+      compileGroup,
+      compileBackendClass,
+      serverId,
+      previous,
+      callback
+    ) {
+      if (!clsiCookiesEnabled) {
+        return callback()
+      }
+      if (serverId == null) {
+        // We don't get a cookie back if it hasn't changed
+        return rclient.expire(
+          this.buildKey(projectId, userId),
+          this._getTTLInSeconds(previous),
+          err => callback(err)
+        )
+      }
+      if (!previous) {
+        // Initial assignment of a user+project or after clearing cache.
+        Metrics.inc('clsi-lb-assign-initial-backend')
+      } else {
+        this.checkIsLoadSheddingEvent(
+          previous,
+          compileGroup,
+          compileBackendClass
+        )
+      }
+      if (rclientSecondary != null) {
+        this._setServerIdInRedis(
+          rclientSecondary,
+          projectId,
+          userId,
+          serverId,
+          () => {}
+        )
+      }
+      this._setServerIdInRedis(rclient, projectId, userId, serverId, err =>
+        callback(err)
+      )
+    },
+
+    _setServerIdInRedis(rclient, projectId, userId, serverId, callback) {
+      rclient.setex(
+        this.buildKey(projectId, userId),
+        this._getTTLInSeconds(serverId),
+        serverId,
+        callback
+      )
+    },
+
+    clearServerId(projectId, userId, callback) {
+      if (!clsiCookiesEnabled) {
+        return callback()
+      }
+      rclient.del(this.buildKey(projectId, userId), err => {
+        if (err) {
+          // redis errors need wrapping as the instance may be shared
+          return callback(
+            new OError(
+              'Failed to clear clsi persistence',
+              { projectId, userId },
+              err
+            )
+          )
+        } else {
+          return callback()
+        }
+      })
+    },
+
+    getCookieJar(
+      projectId,
+      userId,
+      compileGroup,
+      compileBackendClass,
+      callback
+    ) {
+      if (!clsiCookiesEnabled) {
+        return callback(null, request.jar(), undefined)
+      }
+      this.getServerId(
         projectId,
         userId,
         compileGroup,
         compileBackendClass,
-        serverId,
-        null
-      )
-      return serverId
-    } catch (err) {
-      logger.warn(
-        { err, projectId },
-        'error setting server id via populate request'
-      )
-      throw err
-    }
-  }
-
-  function _parseServerIdFromResponse(response) {
-    const cookies = Cookie.parse(response.headers['set-cookie']?.[0] || '')
-    return cookies?.[Settings.clsiCookie.key]
-  }
-
-  async function checkIsLoadSheddingEvent(
-    clsiserverid,
-    compileGroup,
-    compileBackendClass
-  ) {
-    let status
-    try {
-      const { response, body } = await fetchStringWithResponse(
-        `${Settings.apis.clsi.url}/instance-state`,
-        {
-          method: 'GET',
-          query: { clsiserverid, compileGroup, compileBackendClass },
-          signal: AbortSignal.timeout(30_000),
+        (err, serverId) => {
+          if (err != null) {
+            OError.tag(err, 'error getting server id', {
+              project_id: projectId,
+            })
+            return callback(err)
+          }
+          const serverCookie = request.cookie(
+            `${Settings.clsiCookie.key}=${serverId}`
+          )
+          const jar = request.jar()
+          jar.setCookie(serverCookie, Settings.apis.clsi.url)
+          callback(null, jar, serverId)
         }
       )
-      status =
-        response.status === 200 && body === `${clsiserverid},UP\n`
-          ? 'load-shedding'
-          : 'cycle'
-    } catch (err) {
-      if (err instanceof RequestFailedError && err.response.status === 404) {
-        status = 'cycle'
-      } else {
-        status = 'error'
-        logger.warn({ err, clsiserverid }, 'cannot probe clsi VM')
-      }
-    }
-    Metrics.inc('clsi-lb-switch-backend', 1, { status })
-  }
-
-  function _getTTLInSeconds(clsiServerId) {
-    return (clsiServerId || '').includes('-reg-')
-      ? Settings.clsiCookie.ttlInSecondsRegular
-      : Settings.clsiCookie.ttlInSeconds
-  }
-
-  async function setServerId(
-    projectId,
-    userId,
-    compileGroup,
-    compileBackendClass,
-    serverId,
-    previous
-  ) {
-    if (!clsiCookiesEnabled) {
-      return
-    }
-    if (serverId == null) {
-      // We don't get a cookie back if it hasn't changed
-      return await rclient.expire(
-        buildKey(projectId, userId),
-        _getTTLInSeconds(previous)
-      )
-    }
-    if (!previous) {
-      // Initial assignment of a user+project or after clearing cache.
-      Metrics.inc('clsi-lb-assign-initial-backend')
-    } else {
-      await checkIsLoadSheddingEvent(
-        previous,
-        compileGroup,
-        compileBackendClass
-      )
-    }
-    if (rclientSecondary != null) {
-      await _setServerIdInRedis(
-        rclientSecondary,
-        projectId,
-        userId,
-        serverId
-      ).catch(() => {})
-    }
-    await _setServerIdInRedis(rclient, projectId, userId, serverId)
-  }
-
-  async function _setServerIdInRedis(rclient, projectId, userId, serverId) {
-    await rclient.setex(
-      buildKey(projectId, userId),
-      _getTTLInSeconds(serverId),
-      serverId
-    )
-  }
-
-  async function clearServerId(projectId, userId) {
-    if (!clsiCookiesEnabled) {
-      return
-    }
-    try {
-      await rclient.del(buildKey(projectId, userId))
-    } catch (err) {
-      // redis errors need wrapping as the instance may be shared
-      throw new OError(
-        'Failed to clear clsi persistence',
-        { projectId, userId },
-        err
-      )
-    }
-  }
-
-  const cookieManager = {
-    _parseServerIdFromResponse,
-    promises: {
-      getServerId,
-      clearServerId,
-      _populateServerIdViaRequest,
-      setServerId,
     },
   }
-
+  cookieManager.promises = promisifyAll(cookieManager, {
+    without: [
+      '_parseServerIdFromResponse',
+      'checkIsLoadSheddingEvent',
+      '_getTTLInSeconds',
+    ],
+    multiResult: {
+      getCookieJar: ['jar', 'clsiServerId'],
+    },
+  })
   return cookieManager
 }
-
-module.exports = ClsiCookieManagerFactory

services/web/app/src/router.mjs

@@ -1187,9 +1187,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
       const sendRes = _.once(function (statusCode, message) {
         res.status(statusCode)
         plainTextResponse(res, message)
-        ClsiCookieManager.promises
-          .clearServerId(projectId, testUserId)
-          .catch(() => {})
+        ClsiCookieManager.clearServerId(projectId, testUserId, () => {})
       }) // force every compile to a new server
       // set a timeout
       let handler = setTimeout(function () {

services/web/test/unit/src/Compile/ClsiCookieManagerTests.js

@@ -1,19 +1,25 @@
 const sinon = require('sinon')
-const { expect } = require('chai')
+const { assert, expect } = require('chai')
 const modulePath = '../../../../app/src/Features/Compile/ClsiCookieManager.js'
 const SandboxedModule = require('sandboxed-module')
+const realRequst = require('request')
 
 describe('ClsiCookieManager', function () {
   beforeEach(function () {
     this.redis = {
       auth() {},
       get: sinon.stub(),
-      setex: sinon.stub().resolves(),
+      setex: sinon.stub().callsArg(3),
     }
     this.project_id = '123423431321-proj-id'
     this.user_id = 'abc-user-id'
-    this.fetchUtils = {
-      fetchNothing: sinon.stub().returns(Promise.resolve()),
+    this.request = {
+      post: sinon.stub(),
+      cookie: realRequst.cookie,
+      jar: realRequst.jar,
+      defaults: () => {
+        return this.request
+      },
     }
     this.settings = {
       redis: {
@@ -35,7 +41,7 @@ describe('ClsiCookieManager', function () {
         client: () => this.redis,
       }),
       '@overleaf/settings': this.settings,
-      '@overleaf/fetch-utils': this.fetchUtils,
+      request: this.request,
     }
     this.ClsiCookieManager = SandboxedModule.require(modulePath, {
       requires: this.requires,
@@ -43,56 +49,74 @@ describe('ClsiCookieManager', function () {
   })
 
   describe('getServerId', function () {
-    it('should call get for the key', async function () {
-      this.redis.get.resolves('clsi-7')
-      const serverId = await this.ClsiCookieManager.promises.getServerId(
+    it('should call get for the key', function (done) {
+      this.redis.get.callsArgWith(1, null, 'clsi-7')
+      this.ClsiCookieManager.getServerId(
         this.project_id,
         this.user_id,
         '',
-        'e2'
+        'e2',
+        (err, serverId) => {
+          if (err) {
+            return done(err)
+          }
+          this.redis.get
+            .calledWith(`clsiserver:${this.project_id}:${this.user_id}`)
+            .should.equal(true)
+          serverId.should.equal('clsi-7')
+          done()
+        }
       )
-      this.redis.get
-        .calledWith(`clsiserver:${this.project_id}:${this.user_id}`)
-        .should.equal(true)
-      serverId.should.equal('clsi-7')
     })
 
-    it('should _populateServerIdViaRequest if no key is found', async function () {
-      this.ClsiCookieManager.promises._populateServerIdViaRequest = sinon
+    it('should _populateServerIdViaRequest if no key is found', function (done) {
+      this.ClsiCookieManager._populateServerIdViaRequest = sinon
         .stub()
-        .resolves()
-      this.redis.get.resolves(null)
-      await this.ClsiCookieManager.promises.getServerId(
-        this.project_id,
-        this.user_id,
-        ''
-      )
-      this.ClsiCookieManager.promises._populateServerIdViaRequest
-        .calledWith(this.project_id, this.user_id)
-        .should.equal(true)
-    })
-
-    it('should _populateServerIdViaRequest if no key is blank', async function () {
-      this.ClsiCookieManager.promises._populateServerIdViaRequest = sinon
-        .stub()
-        .resolves(null)
-      this.redis.get.resolves('')
-      await this.ClsiCookieManager.promises.getServerId(
+        .yields(null)
+      this.redis.get.callsArgWith(1, null)
+      this.ClsiCookieManager.getServerId(
         this.project_id,
         this.user_id,
         '',
-        'e2'
+        (err, serverId) => {
+          if (err) {
+            return done(err)
+          }
+          this.ClsiCookieManager._populateServerIdViaRequest
+            .calledWith(this.project_id, this.user_id)
+            .should.equal(true)
+          done()
+        }
+      )
+    })
+
+    it('should _populateServerIdViaRequest if no key is blank', function (done) {
+      this.ClsiCookieManager._populateServerIdViaRequest = sinon
+        .stub()
+        .yields(null)
+      this.redis.get.callsArgWith(1, null, '')
+      this.ClsiCookieManager.getServerId(
+        this.project_id,
+        this.user_id,
+        '',
+        'e2',
+        (err, serverId) => {
+          if (err) {
+            return done(err)
+          }
+          this.ClsiCookieManager._populateServerIdViaRequest
+            .calledWith(this.project_id, this.user_id)
+            .should.equal(true)
+          done()
+        }
       )
-      this.ClsiCookieManager.promises._populateServerIdViaRequest
-        .calledWith(this.project_id, this.user_id)
-        .should.equal(true)
     })
   })
 
   describe('_populateServerIdViaRequest', function () {
     beforeEach(function () {
       this.clsiServerId = 'server-id'
-      this.ClsiCookieManager.promises.setServerId = sinon.stub().resolves()
+      this.ClsiCookieManager.setServerId = sinon.stub().yields()
     })
 
     describe('with a server id in the response', function () {
@@ -104,54 +128,71 @@ describe('ClsiCookieManager', function () {
             ],
           },
         }
-        this.fetchUtils.fetchNothing.returns(this.response)
+        this.request.post.callsArgWith(1, null, this.response)
       })
 
-      it('should make a request to the clsi', async function () {
-        await this.ClsiCookieManager.promises._populateServerIdViaRequest(
+      it('should make a request to the clsi', function (done) {
+        this.ClsiCookieManager._populateServerIdViaRequest(
           this.project_id,
           this.user_id,
           'standard',
-          'e2'
+          'e2',
+          (err, serverId) => {
+            if (err) {
+              return done(err)
+            }
+            const args = this.ClsiCookieManager.setServerId.args[0]
+            args[0].should.equal(this.project_id)
+            args[1].should.equal(this.user_id)
+            args[2].should.equal('standard')
+            args[3].should.equal('e2')
+            args[4].should.deep.equal(this.clsiServerId)
+            done()
+          }
         )
-        const args = this.ClsiCookieManager.promises.setServerId.args[0]
-        args[0].should.equal(this.project_id)
-        args[1].should.equal(this.user_id)
-        args[2].should.equal('standard')
-        args[3].should.equal('e2')
-        args[4].should.deep.equal(this.clsiServerId)
       })
 
-      it('should return the server id', async function () {
-        const serverId =
-          await this.ClsiCookieManager.promises._populateServerIdViaRequest(
-            this.project_id,
-            this.user_id,
-            '',
-            'e2'
-          )
-        serverId.should.equal(this.clsiServerId)
+      it('should return the server id', function (done) {
+        this.ClsiCookieManager._populateServerIdViaRequest(
+          this.project_id,
+          this.user_id,
+          '',
+          'e2',
+          (err, serverId) => {
+            if (err) {
+              return done(err)
+            }
+            serverId.should.equal(this.clsiServerId)
+            done()
+          }
+        )
       })
     })
 
     describe('without a server id in the response', function () {
       beforeEach(function () {
         this.response = { headers: {} }
-        this.fetchUtils.fetchNothing.returns(this.response)
+        this.request.post.yields(null, this.response)
       })
-      it('should not set the server id there is no server id in the response', async function () {
+      it('should not set the server id there is no server id in the response', function (done) {
         this.ClsiCookieManager._parseServerIdFromResponse = sinon
           .stub()
           .returns(null)
-        await this.ClsiCookieManager.promises.setServerId(
+        this.ClsiCookieManager.setServerId(
           this.project_id,
           this.user_id,
           'standard',
           'e2',
           this.clsiServerId,
-          null
+          null,
+          err => {
+            if (err) {
+              return done(err)
+            }
+            this.redis.setex.called.should.equal(false)
+            done()
+          }
         )
-        this.redis.setex.called.should.equal(false)
       })
     })
   })
@@ -164,40 +205,52 @@ describe('ClsiCookieManager', function () {
         .returns('clsi-8')
     })
 
-    it('should set the server id with a ttl', async function () {
-      await this.ClsiCookieManager.promises.setServerId(
+    it('should set the server id with a ttl', function (done) {
+      this.ClsiCookieManager.setServerId(
         this.project_id,
         this.user_id,
         'standard',
         'e2',
         this.clsiServerId,
-        null
-      )
-      this.redis.setex.should.have.been.calledWith(
-        `clsiserver:${this.project_id}:${this.user_id}`,
-        this.settings.clsiCookie.ttlInSeconds,
-        this.clsiServerId
+        null,
+        err => {
+          if (err) {
+            return done(err)
+          }
+          this.redis.setex.should.have.been.calledWith(
+            `clsiserver:${this.project_id}:${this.user_id}`,
+            this.settings.clsiCookie.ttlInSeconds,
+            this.clsiServerId
+          )
+          done()
+        }
       )
     })
 
-    it('should set the server id with the regular ttl for reg instance', async function () {
+    it('should set the server id with the regular ttl for reg instance', function (done) {
       this.clsiServerId = 'clsi-reg-8'
-      await this.ClsiCookieManager.promises.setServerId(
+      this.ClsiCookieManager.setServerId(
         this.project_id,
         this.user_id,
         'standard',
         'e2',
         this.clsiServerId,
-        null
-      )
-      expect(this.redis.setex).to.have.been.calledWith(
-        `clsiserver:${this.project_id}:${this.user_id}`,
-        this.settings.clsiCookie.ttlInSecondsRegular,
-        this.clsiServerId
+        null,
+        err => {
+          if (err) {
+            return done(err)
+          }
+          expect(this.redis.setex).to.have.been.calledWith(
+            `clsiserver:${this.project_id}:${this.user_id}`,
+            this.settings.clsiCookie.ttlInSecondsRegular,
+            this.clsiServerId
+          )
+          done()
+        }
       )
     })
 
-    it('should not set the server id if clsiCookies are not enabled', async function () {
+    it('should not set the server id if clsiCookies are not enabled', function (done) {
       delete this.settings.clsiCookie.key
       this.ClsiCookieManager = SandboxedModule.require(modulePath, {
         globals: {
@@ -205,19 +258,25 @@ describe('ClsiCookieManager', function () {
         },
         requires: this.requires,
       })()
-      await this.ClsiCookieManager.promises.setServerId(
+      this.ClsiCookieManager.setServerId(
         this.project_id,
         this.user_id,
         'standard',
         'e2',
         this.clsiServerId,
-        null
+        null,
+        err => {
+          if (err) {
+            return done(err)
+          }
+          this.redis.setex.called.should.equal(false)
+          done()
+        }
       )
-      this.redis.setex.called.should.equal(false)
     })
 
-    it('should also set in the secondary if secondary redis is enabled', async function () {
-      this.redis_secondary = { setex: sinon.stub().resolves() }
+    it('should also set in the secondary if secondary redis is enabled', function (done) {
+      this.redis_secondary = { setex: sinon.stub().callsArg(3) }
       this.settings.redis.clsi_cookie_secondary = {}
       this.RedisWrapper.client = sinon.stub()
       this.RedisWrapper.client.withArgs('clsi_cookie').returns(this.redis)
@@ -233,18 +292,74 @@ describe('ClsiCookieManager', function () {
       this.ClsiCookieManager._parseServerIdFromResponse = sinon
         .stub()
         .returns('clsi-8')
-      await this.ClsiCookieManager.promises.setServerId(
+      this.ClsiCookieManager.setServerId(
         this.project_id,
         this.user_id,
         'standard',
         'e2',
         this.clsiServerId,
-        null
+        null,
+        err => {
+          if (err) {
+            return done(err)
+          }
+          this.redis_secondary.setex.should.have.been.calledWith(
+            `clsiserver:${this.project_id}:${this.user_id}`,
+            this.settings.clsiCookie.ttlInSeconds,
+            this.clsiServerId
+          )
+          done()
+        }
       )
-      this.redis_secondary.setex.should.have.been.calledWith(
-        `clsiserver:${this.project_id}:${this.user_id}`,
-        this.settings.clsiCookie.ttlInSeconds,
-        this.clsiServerId
+    })
+  })
+
+  describe('getCookieJar', function () {
+    beforeEach(function () {
+      this.ClsiCookieManager.getServerId = sinon.stub().yields(null, 'clsi-11')
+    })
+
+    it('should return a jar with the cookie set populated from redis', function (done) {
+      this.ClsiCookieManager.getCookieJar(
+        this.project_id,
+        this.user_id,
+        '',
+        'e2',
+        (err, jar) => {
+          if (err) {
+            return done(err)
+          }
+          jar._jar.store.idx['clsi.example.com']['/'][
+            this.settings.clsiCookie.key
+          ].key.should.equal
+          jar._jar.store.idx['clsi.example.com']['/'][
+            this.settings.clsiCookie.key
+          ].value.should.equal('clsi-11')
+          done()
+        }
+      )
+    })
+
+    it('should return empty cookie jar if clsiCookies are not enabled', function (done) {
+      delete this.settings.clsiCookie.key
+      this.ClsiCookieManager = SandboxedModule.require(modulePath, {
+        globals: {
+          console,
+        },
+        requires: this.requires,
+      })()
+      this.ClsiCookieManager.getCookieJar(
+        this.project_id,
+        this.user_id,
+        '',
+        'e2',
+        (err, jar) => {
+          if (err) {
+            return done(err)
+          }
+          assert.deepEqual(jar, realRequst.jar())
+          done()
+        }
       )
     })
   })

services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -1,3 +1,4 @@
+/* eslint-disable mocha/handle-done-callback */
 const sinon = require('sinon')
 const { expect } = require('chai')
 const modulePath = '../../../../app/src/Features/Compile/CompileController.js'
@@ -18,15 +19,8 @@ describe('CompileController', function () {
         compileTimeout: 100,
       },
     }
-    this.CompileManager = {
-      promises: {
-        compile: sinon.stub(),
-        getProjectCompileLimits: sinon.stub(),
-      },
-    }
-    this.ClsiManager = {
-      promises: {},
-    }
+    this.CompileManager = { compile: sinon.stub() }
+    this.ClsiManager = {}
     this.UserGetter = { getUser: sinon.stub() }
     this.rateLimiter = {
       consume: sinon.stub().resolves(),
@@ -53,11 +47,10 @@ describe('CompileController', function () {
       },
     }
     this.ClsiCookieManager = {
-      promises: {
-        getServerId: sinon.stub().resolves('clsi-server-id-from-redis'),
-      },
+      getServerId: sinon.stub().yields(null, 'clsi-server-id-from-redis'),
     }
     this.SessionManager = {
+      getLoggedInUser: sinon.stub().callsArgWith(1, null, this.user),
       getLoggedInUserId: sinon.stub().returns(this.user_id),
       getSessionUser: sinon.stub().returns(this.user),
       isUserLoggedIn: sinon.stub().returns(true),
@@ -83,9 +76,8 @@ describe('CompileController', function () {
         'stream/promises': { pipeline: this.pipeline },
         '@overleaf/settings': this.settings,
         '@overleaf/fetch-utils': this.fetchUtils,
-        '../Project/ProjectGetter': (this.ProjectGetter = {
-          promises: {},
-        }),
+        request: (this.request = sinon.stub()),
+        '../Project/ProjectGetter': (this.ProjectGetter = {}),
         '@overleaf/metrics': (this.Metrics = {
           inc: sinon.stub(),
           Timer: class {
@@ -129,23 +121,25 @@ describe('CompileController', function () {
     beforeEach(function () {
       this.req.params = { Project_id: this.projectId }
       this.req.session = {}
-      this.CompileManager.promises.compile = sinon.stub().resolves({
-        status: (this.status = 'success'),
-        outputFiles: (this.outputFiles = [
+      this.CompileManager.compile = sinon.stub().callsArgWith(
+        3,
+        null,
+        (this.status = 'success'),
+        (this.outputFiles = [
           {
             path: 'output.pdf',
             url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
             type: 'pdf',
           },
         ]),
-        clsiServerId: undefined,
-        limits: undefined,
-        validationProblems: undefined,
-        stats: undefined,
-        timings: undefined,
-        outputUrlPrefix: undefined,
-        buildId: this.build_id,
-      })
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        this.build_id
+      )
     })
 
     describe('pdfDownloadDomain', function () {
@@ -154,8 +148,9 @@ describe('CompileController', function () {
       })
 
       describe('when clsi does not emit zone prefix', function () {
-        beforeEach(async function () {
-          await this.CompileController.compile(this.req, this.res, this.next)
+        beforeEach(function (done) {
+          this.res.callback = done
+          this.CompileController.compile(this.req, this.res, this.next)
         })
 
         it('should add domain verbatim', function () {
@@ -182,25 +177,28 @@ describe('CompileController', function () {
       })
 
       describe('when clsi emits a zone prefix', function () {
-        beforeEach(async function () {
-          this.CompileManager.promises.compile = sinon.stub().resolves({
-            status: (this.status = 'success'),
-            outputFiles: (this.outputFiles = [
+        beforeEach(function (done) {
+          this.res.callback = done
+          this.CompileManager.compile = sinon.stub().callsArgWith(
+            3,
+            null,
+            (this.status = 'success'),
+            (this.outputFiles = [
               {
                 path: 'output.pdf',
                 url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
                 type: 'pdf',
               },
             ]),
-            clsiServerId: undefined,
-            limits: undefined,
-            validationProblems: undefined,
-            stats: undefined,
-            timings: undefined,
-            outputUrlPrefix: '/zone/b',
-            buildId: this.build_id,
-          })
-          await this.CompileController.compile(this.req, this.res, this.next)
+            undefined, // clsiServerId
+            undefined, // limits
+            undefined, // validationProblems
+            undefined, // stats
+            undefined, // timings
+            '/zone/b',
+            this.build_id
+          )
+          this.CompileController.compile(this.req, this.res, this.next)
         })
 
         it('should add the zone prefix', function () {
@@ -229,8 +227,9 @@ describe('CompileController', function () {
     })
 
     describe('when not an auto compile', function () {
-      beforeEach(async function () {
-        await this.CompileController.compile(this.req, this.res, this.next)
+      beforeEach(function (done) {
+        this.res.callback = done
+        this.CompileController.compile(this.req, this.res, this.next)
       })
 
       it('should look up the user id', function () {
@@ -240,7 +239,7 @@ describe('CompileController', function () {
       })
 
       it('should do the compile without the auto compile flag', function () {
-        this.CompileManager.promises.compile.should.have.been.calledWith(
+        this.CompileManager.compile.should.have.been.calledWith(
           this.projectId,
           this.user_id,
           {
@@ -276,13 +275,14 @@ describe('CompileController', function () {
     })
 
     describe('when an auto compile', function () {
-      beforeEach(async function () {
+      beforeEach(function (done) {
+        this.res.callback = done
         this.req.query = { auto_compile: 'true' }
-        await this.CompileController.compile(this.req, this.res, this.next)
+        this.CompileController.compile(this.req, this.res, this.next)
       })
 
       it('should do the compile with the auto compile flag', function () {
-        this.CompileManager.promises.compile.should.have.been.calledWith(
+        this.CompileManager.compile.should.have.been.calledWith(
           this.projectId,
           this.user_id,
           {
@@ -299,13 +299,14 @@ describe('CompileController', function () {
     })
 
     describe('with the draft attribute', function () {
-      beforeEach(async function () {
+      beforeEach(function (done) {
+        this.res.callback = done
         this.req.body = { draft: true }
-        await this.CompileController.compile(this.req, this.res, this.next)
+        this.CompileController.compile(this.req, this.res, this.next)
       })
 
       it('should do the compile without the draft compile flag', function () {
-        this.CompileManager.promises.compile.should.have.been.calledWith(
+        this.CompileManager.compile.should.have.been.calledWith(
           this.projectId,
           this.user_id,
           {
@@ -323,13 +324,14 @@ describe('CompileController', function () {
     })
 
     describe('with an editor id', function () {
-      beforeEach(async function () {
+      beforeEach(function (done) {
+        this.res.callback = done
         this.req.body = { editorId: 'the-editor-id' }
-        await this.CompileController.compile(this.req, this.res, this.next)
+        this.CompileController.compile(this.req, this.res, this.next)
       })
 
       it('should pass the editor id to the compiler', function () {
-        this.CompileManager.promises.compile.should.have.been.calledWith(
+        this.CompileManager.compile.should.have.been.calledWith(
           this.projectId,
           this.user_id,
           {
@@ -351,29 +353,25 @@ describe('CompileController', function () {
       this.submission_id = 'sub-1234'
       this.req.params = { submission_id: this.submission_id }
       this.req.body = {}
-      this.ClsiManager.promises.sendExternalRequest = sinon.stub().resolves({
-        status: (this.status = 'success'),
-        outputFiles: (this.outputFiles = ['mock-output-files']),
-        clsiServerId: 'mock-server-id',
-        validationProblems: null,
-      })
+      this.ClsiManager.sendExternalRequest = sinon
+        .stub()
+        .callsArgWith(
+          3,
+          null,
+          (this.status = 'success'),
+          (this.outputFiles = ['mock-output-files']),
+          (this.clsiServerId = 'mock-server-id'),
+          (this.validationProblems = null)
+        )
     })
 
-    it('should set the content-type of the response to application/json', async function () {
-      await this.CompileController.compileSubmission(
-        this.req,
-        this.res,
-        this.next
-      )
+    it('should set the content-type of the response to application/json', function () {
+      this.CompileController.compileSubmission(this.req, this.res, this.next)
       this.res.contentType.calledWith('application/json').should.equal(true)
     })
 
-    it('should send a successful response reporting the status and files', async function () {
-      await this.CompileController.compileSubmission(
-        this.req,
-        this.res,
-        this.next
-      )
+    it('should send a successful response reporting the status and files', function () {
+      this.CompileController.compileSubmission(this.req, this.res, this.next)
       this.res.statusCode.should.equal(200)
       this.res.body.should.equal(
         JSON.stringify({
@@ -395,7 +393,7 @@ describe('CompileController', function () {
       })
 
       it('should use the supplied values', function () {
-        this.ClsiManager.promises.sendExternalRequest.should.have.been.calledWith(
+        this.ClsiManager.sendExternalRequest.should.have.been.calledWith(
           this.submission_id,
           { compileGroup: 'special', timeout: 600 },
           { compileGroup: 'special', compileBackendClass: 'n2d', timeout: 600 }
@@ -415,7 +413,7 @@ describe('CompileController', function () {
       })
 
       it('should use the other options but default values for compileGroup and timeout', function () {
-        this.ClsiManager.promises.sendExternalRequest.should.have.been.calledWith(
+        this.ClsiManager.sendExternalRequest.should.have.been.calledWith(
           this.submission_id,
           {
             rootResourcePath: 'main.tex',
@@ -439,21 +437,24 @@ describe('CompileController', function () {
 
   describe('downloadPdf', function () {
     beforeEach(function () {
-      this.CompileController._proxyToClsi = sinon.stub().resolves()
       this.req.params = { Project_id: this.projectId }
+
       this.project = { name: 'test namè; 1' }
-      this.ProjectGetter.promises.getProject = sinon
+      this.ProjectGetter.getProject = sinon
         .stub()
-        .resolves(this.project)
+        .callsArgWith(2, null, this.project)
     })
 
     describe('when downloading for embedding', function () {
-      beforeEach(async function () {
-        await this.CompileController.downloadPdf(this.req, this.res, this.next)
+      beforeEach(function (done) {
+        this.CompileController.proxyToClsi = sinon
+          .stub()
+          .callsFake(() => done())
+        this.CompileController.downloadPdf(this.req, this.res, this.next)
       })
 
       it('should look up the project', function () {
-        this.ProjectGetter.promises.getProject
+        this.ProjectGetter.getProject
           .calledWith(this.projectId, { name: 1 })
           .should.equal(true)
       })
@@ -473,66 +474,43 @@ describe('CompileController', function () {
       })
 
       it('should proxy the PDF from the CLSI', function () {
-        this.CompileController._proxyToClsi
+        this.CompileController.proxyToClsi
           .calledWith(
             this.projectId,
             'output-file',
             `/project/${this.projectId}/user/${this.user_id}/output/output.pdf`,
             {},
             this.req,
-            this.res
+            this.res,
+            this.next
           )
           .should.equal(true)
       })
     })
 
     describe('when a build-id is provided', function () {
-      beforeEach(async function () {
+      beforeEach(function (done) {
         this.req.params.build_id = this.build_id
-        await this.CompileController.downloadPdf(this.req, this.res, this.next)
+        this.CompileController.proxyToClsi = sinon
+          .stub()
+          .callsFake(() => done())
+        this.CompileController.downloadPdf(this.req, this.res, this.next)
       })
 
       it('should proxy the PDF from the CLSI, with a build-id', function () {
-        this.CompileController._proxyToClsi
+        this.CompileController.proxyToClsi
           .calledWith(
             this.projectId,
             'output-file',
             `/project/${this.projectId}/user/${this.user_id}/build/${this.build_id}/output/output.pdf`,
             {},
             this.req,
-            this.res
+            this.res,
+            this.next
           )
           .should.equal(true)
       })
     })
-
-    describe('when rate-limited', function () {
-      beforeEach(async function () {
-        this.rateLimiter.consume.rejects({
-          msBeforeNext: 250,
-          remainingPoints: 0,
-          consumedPoints: 5,
-          isFirstInDuration: false,
-        })
-      })
-      it('should return 500', async function () {
-        await this.CompileController.downloadPdf(this.req, this.res, this.next)
-        // should it be 429 instead?
-        this.res.sendStatus.calledWith(500).should.equal(true)
-        this.CompileController._proxyToClsi.should.not.have.been.called
-      })
-    })
-
-    describe('when rate-limit errors', function () {
-      beforeEach(async function () {
-        this.rateLimiter.consume.rejects(new Error('uh oh'))
-      })
-      it('should return 500', async function () {
-        await this.CompileController.downloadPdf(this.req, this.res, this.next)
-        this.res.sendStatus.calledWith(500).should.equal(true)
-        this.CompileController._proxyToClsi.should.not.have.been.called
-      })
-    })
   })
 
   describe('getFileFromClsiWithoutUser', function () {
@@ -546,12 +524,12 @@ describe('CompileController', function () {
       }
       this.req.body = {}
       this.expected_url = `/project/${this.submission_id}/build/${this.build_id}/output/${this.file}`
-      this.CompileController._proxyToClsiWithLimits = sinon.stub()
+      this.CompileController.proxyToClsiWithLimits = sinon.stub()
     })
 
     describe('without limits specified', function () {
-      beforeEach(async function () {
-        await this.CompileController.getFileFromClsiWithoutUser(
+      beforeEach(function () {
+        this.CompileController.getFileFromClsiWithoutUser(
           this.req,
           this.res,
           this.next
@@ -559,12 +537,15 @@ describe('CompileController', function () {
       })
 
       it('should proxy to CLSI with correct URL and default limits', function () {
-        this.CompileController._proxyToClsiWithLimits.should.have.been.calledWith(
+        this.CompileController.proxyToClsiWithLimits.should.have.been.calledWith(
           this.submission_id,
           'output-file',
           this.expected_url,
           {},
-          { compileGroup: 'standard', compileBackendClass: 'n2d' }
+          {
+            compileGroup: 'standard',
+            compileBackendClass: 'n2d',
+          }
         )
       })
     })
@@ -580,7 +561,7 @@ describe('CompileController', function () {
       })
 
       it('should proxy to CLSI with correct URL and specified limits', function () {
-        this.CompileController._proxyToClsiWithLimits.should.have.been.calledWith(
+        this.CompileController.proxyToClsiWithLimits.should.have.been.calledWith(
           this.submission_id,
           'output-file',
           this.expected_url,
@@ -596,7 +577,7 @@ describe('CompileController', function () {
   describe('proxySyncCode', function () {
     let file, line, column, imageName, editorId, buildId
 
-    beforeEach(async function () {
+    beforeEach(function (done) {
       this.req.params = { Project_id: this.projectId }
       file = 'main.tex'
       line = String(Date.now())
@@ -606,17 +587,17 @@ describe('CompileController', function () {
       this.req.query = { file, line, column, editorId, buildId }
 
       imageName = 'foo/bar:tag-0'
-      this.ProjectGetter.promises.getProject = sinon
-        .stub()
-        .resolves({ imageName })
+      this.ProjectGetter.getProject = sinon.stub().yields(null, { imageName })
 
-      this.CompileController._proxyToClsi = sinon.stub().resolves()
+      this.next.callsFake(done)
+      this.res.callback = done
+      this.CompileController.proxyToClsi = sinon.stub().callsFake(() => done())
 
-      await this.CompileController.proxySyncCode(this.req, this.res, this.next)
+      this.CompileController.proxySyncCode(this.req, this.res, this.next)
     })
 
     it('should proxy the request with an imageName', function () {
-      expect(this.CompileController._proxyToClsi).to.have.been.calledWith(
+      expect(this.CompileController.proxyToClsi).to.have.been.calledWith(
         this.projectId,
         'sync-to-code',
         `/project/${this.projectId}/user/${this.user_id}/sync/code`,
@@ -630,7 +611,8 @@ describe('CompileController', function () {
           compileFromClsiCache: false,
         },
         this.req,
-        this.res
+        this.res,
+        this.next
       )
     })
   })
@@ -638,7 +620,7 @@ describe('CompileController', function () {
   describe('proxySyncPdf', function () {
     let page, h, v, imageName, editorId, buildId
 
-    beforeEach(async function () {
+    beforeEach(function (done) {
       this.req.params = { Project_id: this.projectId }
       page = String(Date.now())
       h = String(Math.random())
@@ -648,17 +630,17 @@ describe('CompileController', function () {
       this.req.query = { page, h, v, editorId, buildId }
 
       imageName = 'foo/bar:tag-1'
-      this.ProjectGetter.promises.getProject = sinon
-        .stub()
-        .resolves({ imageName })
+      this.ProjectGetter.getProject = sinon.stub().yields(null, { imageName })
 
-      this.CompileController._proxyToClsi = sinon.stub()
+      this.next.callsFake(done)
+      this.res.callback = done
+      this.CompileController.proxyToClsi = sinon.stub().callsFake(() => done())
 
-      await this.CompileController.proxySyncPdf(this.req, this.res, this.next)
+      this.CompileController.proxySyncPdf(this.req, this.res, this.next)
     })
 
     it('should proxy the request with an imageName', function () {
-      expect(this.CompileController._proxyToClsi).to.have.been.calledWith(
+      expect(this.CompileController.proxyToClsi).to.have.been.calledWith(
         this.projectId,
         'sync-to-pdf',
         `/project/${this.projectId}/user/${this.user_id}/sync/pdf`,
@@ -672,12 +654,13 @@ describe('CompileController', function () {
           compileFromClsiCache: false,
         },
         this.req,
-        this.res
+        this.res,
+        this.next
       )
     })
   })
 
-  describe('_proxyToClsi', function () {
+  describe('proxyToClsi', function () {
     beforeEach(function () {
       this.req.method = 'mock-method'
       this.req.headers = {
@@ -690,14 +673,15 @@ describe('CompileController', function () {
 
     describe('old pdf viewer', function () {
       describe('user with standard priority', function () {
-        beforeEach(async function () {
-          this.CompileManager.promises.getProjectCompileLimits = sinon
+        beforeEach(function (done) {
+          this.res.callback = done
+          this.CompileManager.getProjectCompileLimits = sinon
             .stub()
-            .resolves({
+            .callsArgWith(1, null, {
               compileGroup: 'standard',
               compileBackendClass: 'e2',
             })
-          await this.CompileController._proxyToClsi(
+          this.CompileController.proxyToClsi(
             this.projectId,
             'output-file',
             (this.url = '/test'),
@@ -720,14 +704,15 @@ describe('CompileController', function () {
       })
 
       describe('user with priority compile', function () {
-        beforeEach(async function () {
-          this.CompileManager.promises.getProjectCompileLimits = sinon
+        beforeEach(function (done) {
+          this.res.callback = done
+          this.CompileManager.getProjectCompileLimits = sinon
             .stub()
-            .resolves({
+            .callsArgWith(1, null, {
               compileGroup: 'priority',
               compileBackendClass: 'c2d',
             })
-          await this.CompileController._proxyToClsi(
+          this.CompileController.proxyToClsi(
             this.projectId,
             'output-file',
             (this.url = '/test'),
@@ -746,15 +731,16 @@ describe('CompileController', function () {
       })
 
       describe('user with standard priority via query string', function () {
-        beforeEach(async function () {
+        beforeEach(function (done) {
+          this.res.callback = done
           this.req.query = { compileGroup: 'standard' }
-          this.CompileManager.promises.getProjectCompileLimits = sinon
+          this.CompileManager.getProjectCompileLimits = sinon
             .stub()
-            .resolves({
+            .callsArgWith(1, null, {
               compileGroup: 'standard',
               compileBackendClass: 'e2',
             })
-          await this.CompileController._proxyToClsi(
+          this.CompileController.proxyToClsi(
             this.projectId,
             'output-file',
             (this.url = '/test'),
@@ -777,15 +763,16 @@ describe('CompileController', function () {
       })
 
       describe('user with non-existent priority via query string', function () {
-        beforeEach(async function () {
+        beforeEach(function (done) {
+          this.res.callback = done
           this.req.query = { compileGroup: 'foobar' }
-          this.CompileManager.promises.getProjectCompileLimits = sinon
+          this.CompileManager.getProjectCompileLimits = sinon
             .stub()
-            .resolves({
+            .callsArgWith(1, null, {
               compileGroup: 'standard',
               compileBackendClass: 'e2',
             })
-          await this.CompileController._proxyToClsi(
+          this.CompileController.proxyToClsi(
             this.projectId,
             'output-file',
             (this.url = '/test'),
@@ -804,15 +791,16 @@ describe('CompileController', function () {
       })
 
       describe('user with build parameter via query string', function () {
-        beforeEach(async function () {
-          this.CompileManager.promises.getProjectCompileLimits = sinon
+        beforeEach(function (done) {
+          this.res.callback = done
+          this.CompileManager.getProjectCompileLimits = sinon
             .stub()
-            .resolves({
+            .callsArgWith(1, null, {
               compileGroup: 'standard',
               compileBackendClass: 'e2',
             })
           this.req.query = { build: 1234 }
-          await this.CompileController._proxyToClsi(
+          this.CompileController.proxyToClsi(
             this.projectId,
             'output-file',
             (this.url = '/test'),
@@ -833,16 +821,16 @@ describe('CompileController', function () {
   })
 
   describe('deleteAuxFiles', function () {
-    beforeEach(async function () {
-      this.CompileManager.promises.deleteAuxFiles = sinon.stub().resolves()
+    beforeEach(function () {
+      this.CompileManager.deleteAuxFiles = sinon.stub().yields()
       this.req.params = { Project_id: this.projectId }
       this.req.query = { clsiserverid: 'node-1' }
       this.res.sendStatus = sinon.stub()
-      await this.CompileController.deleteAuxFiles(this.req, this.res, this.next)
+      this.CompileController.deleteAuxFiles(this.req, this.res, this.next)
     })
 
     it('should proxy to the CLSI', function () {
-      this.CompileManager.promises.deleteAuxFiles
+      this.CompileManager.deleteAuxFiles
         .calledWith(this.projectId, this.user_id, 'node-1')
         .should.equal(true)
     })
@@ -860,25 +848,26 @@ describe('CompileController', function () {
         },
       }
       this.downloadPath = `/project/${this.projectId}/build/123/output/output.pdf`
-      this.CompileManager.promises.compile.resolves({
-        status: 'success',
-        outputFiles: [{ path: 'output.pdf', url: this.downloadPath }],
-      })
-      this.CompileController._proxyToClsi = sinon.stub()
+      this.CompileManager.compile.callsArgWith(3, null, 'success', [
+        {
+          path: 'output.pdf',
+          url: this.downloadPath,
+        },
+      ])
+      this.CompileController.proxyToClsi = sinon.stub()
       this.res = { send: () => {}, sendStatus: sinon.stub() }
     })
 
-    it('should call compile in the compile manager', async function () {
-      await this.CompileController.compileAndDownloadPdf(this.req, this.res)
-      this.CompileManager.promises.compile
-        .calledWith(this.projectId)
-        .should.equal(true)
+    it('should call compile in the compile manager', function (done) {
+      this.CompileController.compileAndDownloadPdf(this.req, this.res)
+      this.CompileManager.compile.calledWith(this.projectId).should.equal(true)
+      done()
     })
 
-    it('should proxy the res to the clsi with correct url', async function () {
-      await this.CompileController.compileAndDownloadPdf(this.req, this.res)
+    it('should proxy the res to the clsi with correct url', function (done) {
+      this.CompileController.compileAndDownloadPdf(this.req, this.res)
       sinon.assert.calledWith(
-        this.CompileController._proxyToClsi,
+        this.CompileController.proxyToClsi,
         this.projectId,
         'output-file',
         this.downloadPath,
@@ -887,7 +876,7 @@ describe('CompileController', function () {
         this.res
       )
 
-      this.CompileController._proxyToClsi
+      this.CompileController.proxyToClsi
         .calledWith(
           this.projectId,
           'output-file',
@@ -897,44 +886,38 @@ describe('CompileController', function () {
           this.res
         )
         .should.equal(true)
+      done()
     })
 
-    it('should not download anything on compilation failures', async function () {
-      this.CompileManager.promises.compile.rejects(new Error('failed'))
-      await this.CompileController.compileAndDownloadPdf(
-        this.req,
-        this.res,
-        this.next
-      )
+    it('should not download anything on compilation failures', function () {
+      this.CompileManager.compile.yields(new Error('failed'))
+      this.CompileController.compileAndDownloadPdf(this.req, this.res)
       this.res.sendStatus.should.have.been.calledWith(500)
-      this.CompileController._proxyToClsi.should.not.have.been.called
+      this.CompileController.proxyToClsi.should.not.have.been.called
     })
 
-    it('should not download anything on missing pdf', async function () {
-      this.CompileManager.promises.compile.resolves({
-        status: 'success',
-        outputFiles: [],
-      })
-      await this.CompileController.compileAndDownloadPdf(this.req, this.res)
+    it('should not download anything on missing pdf', function () {
+      this.CompileManager.compile.yields(null, 'success', [])
+      this.CompileController.compileAndDownloadPdf(this.req, this.res)
       this.res.sendStatus.should.have.been.calledWith(500)
-      this.CompileController._proxyToClsi.should.not.have.been.called
+      this.CompileController.proxyToClsi.should.not.have.been.called
     })
   })
 
   describe('wordCount', function () {
-    beforeEach(async function () {
-      this.CompileManager.promises.wordCount = sinon
+    beforeEach(function () {
+      this.CompileManager.wordCount = sinon
         .stub()
-        .resolves({ content: 'body' })
+        .yields(null, { content: 'body' })
       this.req.params = { Project_id: this.projectId }
       this.req.query = { clsiserverid: 'node-42' }
       this.res.json = sinon.stub()
       this.res.contentType = sinon.stub()
-      await this.CompileController.wordCount(this.req, this.res, this.next)
+      this.CompileController.wordCount(this.req, this.res, this.next)
     })
 
     it('should proxy to the CLSI', function () {
-      this.CompileManager.promises.wordCount
+      this.CompileManager.wordCount
         .calledWith(this.projectId, this.user_id, false, 'node-42')
         .should.equal(true)
     })