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

Revert "[web] Promisify ClsiCookieManager and CompileController" GitOrigin-RevId: 190ee8d2be23687f092e762c5199a34bcdf37cf9
2025-05-01 08:06:00 +00:00
parent 7256c99e29
commit d7d60f9d4c
5 changed files with 1061 additions and 879 deletions
@@ -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
@@ -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 () {