Merge pull request #24754 from overleaf/em-promisify-history-controller

Promisify HistoryController

GitOrigin-RevId: e1783acb8c7ba7e00b109a4f4a514973cc3163d2

services/web/app/src/Features/History/HistoryController.js

@@ -1,8 +1,17 @@
-let HistoryController
+// @ts-check
+
+const { setTimeout } = require('timers/promises')
+const { pipeline } = require('stream/promises')
 const OError = require('@overleaf/o-error')
-const async = require('async')
 const logger = require('@overleaf/logger')
-const request = require('request')
+const { expressify } = require('@overleaf/promise-utils')
+const {
+  fetchStream,
+  fetchStreamWithResponse,
+  fetchJson,
+  fetchNothing,
+  RequestFailedError,
+} = require('@overleaf/fetch-utils')
 const settings = require('@overleaf/settings')
 const SessionManager = require('../Authentication/SessionManager')
 const UserGetter = require('../User/UserGetter')
@@ -12,11 +21,8 @@ const HistoryManager = require('./HistoryManager')
 const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler')
 const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler')
 const RestoreManager = require('./RestoreManager')
-const { pipeline } = require('stream')
-const Stream = require('stream')
 const { prepareZipAttachment } = require('../../infrastructure/Response')
 const Features = require('../../infrastructure/Features')
-const { expressify } = require('@overleaf/promise-utils')
 
 // Number of seconds after which the browser should send a request to revalidate
 // blobs
@@ -26,6 +32,8 @@ const REVALIDATE_BLOB_AFTER_SECONDS = 86400 // 1 day
 // revalidating
 const STALE_WHILE_REVALIDATE_SECONDS = 365 * 86400 // 1 year
 
+const MAX_HISTORY_ZIP_ATTEMPTS = 40
+
 async function getBlob(req, res) {
   await requestBlob('GET', req, res)
 }
@@ -44,9 +52,9 @@ async function requestBlob(method, req, res) {
   }
 
   const range = req.get('Range')
-  let url, stream, source, contentLength
+  let stream, source, contentLength
   try {
-    ;({ url, stream, source, contentLength } =
+    ;({ stream, source, contentLength } =
       await HistoryManager.promises.requestBlobWithFallback(
         projectId,
         hash,
@@ -65,14 +73,13 @@ async function requestBlob(method, req, res) {
   setBlobCacheHeaders(res, hash)
 
   try {
-    await Stream.promises.pipeline(stream, res)
+    await pipeline(stream, res)
   } catch (err) {
     // If the downstream request is cancelled, we get an
     // ERR_STREAM_PREMATURE_CLOSE, ignore these "errors".
-    if (err?.code === 'ERR_STREAM_PREMATURE_CLOSE') return
-
-    logger.warn({ err, url, method, range }, 'streaming blob error')
-    throw err
+    if (!isPrematureClose(err)) {
+      throw err
+    }
   }
 }
 
@@ -88,492 +95,393 @@ function setBlobCacheHeaders(res, etag) {
   res.set('ETag', etag)
 }
 
-module.exports = HistoryController = {
-  getBlob: expressify(getBlob),
-  headBlob: expressify(headBlob),
-
-  proxyToHistoryApi(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const url = settings.apis.project_history.url + req.url
+async function proxyToHistoryApi(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const url = settings.apis.project_history.url + req.url
 
-    const getReq = request({
-      url,
-      method: req.method,
-      headers: {
-        'X-User-Id': userId,
-      },
-    })
-    pipeline(getReq, res, function (err) {
-      // If the downstream request is cancelled, we get an
-      // ERR_STREAM_PREMATURE_CLOSE.
-      if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
-        logger.warn({ url, err }, 'history API error')
-        next(err)
-      }
-    })
-  },
+  const { stream, response } = await fetchStreamWithResponse(url, {
+    method: req.method,
+    headers: { 'X-User-Id': userId },
+  })
 
-  proxyToHistoryApiAndInjectUserDetails(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const url = settings.apis.project_history.url + req.url
-    HistoryController._makeRequest(
-      {
-        url,
-        method: req.method,
-        json: true,
-        headers: {
-          'X-User-Id': userId,
-        },
-      },
-      function (err, body) {
-        if (err) {
-          return next(err)
-        }
-        HistoryManager.injectUserDetails(body, function (err, data) {
-          if (err) {
-            return next(err)
-          }
-          res.json(data)
-        })
-      }
-    )
-  },
+  const contentType = response.headers.get('Content-Type')
+  const contentLength = response.headers.get('Content-Length')
+  if (contentType != null) {
+    res.set('Content-Type', contentType)
+  }
+  if (contentLength != null) {
+    res.set('Content-Length', contentLength)
+  }
 
-  resyncProjectHistory(req, res, next) {
-    // increase timeout to 6 minutes
-    res.setTimeout(6 * 60 * 1000)
-    const projectId = req.params.Project_id
-    const opts = {}
-    const historyRangesMigration = req.body.historyRangesMigration
-    if (historyRangesMigration) {
-      opts.historyRangesMigration = historyRangesMigration
-    }
-    if (req.body.resyncProjectStructureOnly) {
-      opts.resyncProjectStructureOnly = req.body.resyncProjectStructureOnly
+  try {
+    await pipeline(stream, res)
+  } catch (err) {
+    // If the downstream request is cancelled, we get an
+    // ERR_STREAM_PREMATURE_CLOSE.
+    if (!isPrematureClose(err)) {
+      throw err
     }
-    ProjectEntityUpdateHandler.resyncProjectHistory(
-      projectId,
-      opts,
-      function (err) {
-        if (err instanceof Errors.ProjectHistoryDisabledError) {
-          return res.sendStatus(404)
-        }
-        if (err) {
-          return next(err)
-        }
-        res.sendStatus(204)
-      }
-    )
-  },
+  }
+}
 
-  restoreFileFromV2(req, res, next) {
-    const { project_id: projectId } = req.params
-    const { version, pathname } = req.body
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    RestoreManager.restoreFileFromV2(
-      userId,
-      projectId,
-      version,
-      pathname,
-      function (err, entity) {
-        if (err) {
-          return next(err)
-        }
-        res.json({
-          type: entity.type,
-          id: entity._id,
-        })
-      }
-    )
-  },
+async function proxyToHistoryApiAndInjectUserDetails(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const url = settings.apis.project_history.url + req.url
+  const body = await fetchJson(url, {
+    method: req.method,
+    headers: { 'X-User-Id': userId },
+  })
+  const data = await HistoryManager.promises.injectUserDetails(body)
+  res.json(data)
+}
+
+async function resyncProjectHistory(req, res, next) {
+  // increase timeout to 6 minutes
+  res.setTimeout(6 * 60 * 1000)
+  const projectId = req.params.Project_id
+  const opts = {}
+  const historyRangesMigration = req.body.historyRangesMigration
+  if (historyRangesMigration) {
+    opts.historyRangesMigration = historyRangesMigration
+  }
+  if (req.body.resyncProjectStructureOnly) {
+    opts.resyncProjectStructureOnly = req.body.resyncProjectStructureOnly
+  }
 
-  revertFile(req, res, next) {
-    const { project_id: projectId } = req.params
-    const { version, pathname } = req.body
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    RestoreManager.revertFile(
-      userId,
+  try {
+    await ProjectEntityUpdateHandler.promises.resyncProjectHistory(
       projectId,
-      version,
-      pathname,
-      {},
-      function (err, entity) {
-        if (err) {
-          return next(err)
-        }
-        res.json({
-          type: entity.type,
-          id: entity._id,
-        })
-      }
+      opts
     )
-  },
+  } catch (err) {
+    if (err instanceof Errors.ProjectHistoryDisabledError) {
+      return res.sendStatus(404)
+    } else {
+      throw err
+    }
+  }
 
-  revertProject(req, res, next) {
-    const { project_id: projectId } = req.params
-    const { version } = req.body
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    RestoreManager.revertProject(userId, projectId, version, function (err) {
-      if (err) {
-        return next(err)
-      }
-      res.sendStatus(200)
-    })
-  },
+  res.sendStatus(204)
+}
 
-  getLabels(req, res, next) {
-    const projectId = req.params.Project_id
-    HistoryController._makeRequest(
-      {
-        method: 'GET',
-        url: `${settings.apis.project_history.url}/project/${projectId}/labels`,
-        json: true,
-      },
-      function (err, labels) {
-        if (err) {
-          return next(err)
-        }
-        HistoryController._enrichLabels(labels, (err, labels) => {
-          if (err) {
-            return next(err)
-          }
-          res.json(labels)
-        })
-      }
-    )
-  },
+async function restoreFileFromV2(req, res, next) {
+  const { project_id: projectId } = req.params
+  const { version, pathname } = req.body
+  const userId = SessionManager.getLoggedInUserId(req.session)
 
-  createLabel(req, res, next) {
-    const projectId = req.params.Project_id
-    const { comment, version } = req.body
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    HistoryController._makeRequest(
-      {
-        method: 'POST',
-        url: `${settings.apis.project_history.url}/project/${projectId}/labels`,
-        json: { comment, version, user_id: userId },
-      },
-      function (err, label) {
-        if (err) {
-          return next(err)
-        }
-        HistoryController._enrichLabel(label, (err, label) => {
-          if (err) {
-            return next(err)
-          }
-          res.json(label)
-        })
-      }
-    )
-  },
+  const entity = await RestoreManager.promises.restoreFileFromV2(
+    userId,
+    projectId,
+    version,
+    pathname
+  )
 
-  _enrichLabel(label, callback) {
-    if (!label.user_id) {
-      const newLabel = Object.assign({}, label)
-      newLabel.user_display_name = HistoryController._displayNameForUser(null)
-      return callback(null, newLabel)
-    }
-    UserGetter.getUser(
-      label.user_id,
-      { first_name: 1, last_name: 1, email: 1 },
-      (err, user) => {
-        if (err) {
-          return callback(err)
-        }
-        const newLabel = Object.assign({}, label)
-        newLabel.user_display_name = HistoryController._displayNameForUser(user)
-        callback(null, newLabel)
-      }
-    )
-  },
+  res.json({
+    type: entity.type,
+    id: entity._id,
+  })
+}
 
-  _enrichLabels(labels, callback) {
-    if (!labels || !labels.length) {
-      return callback(null, [])
-    }
-    const uniqueUsers = new Set(labels.map(label => label.user_id))
+async function revertFile(req, res, next) {
+  const { project_id: projectId } = req.params
+  const { version, pathname } = req.body
+  const userId = SessionManager.getLoggedInUserId(req.session)
+
+  const entity = await RestoreManager.promises.revertFile(
+    userId,
+    projectId,
+    version,
+    pathname,
+    {}
+  )
 
-    // For backwards compatibility, and for anonymously created labels in SP
-    // expect missing user_id fields
-    uniqueUsers.delete(undefined)
+  res.json({
+    type: entity.type,
+    id: entity._id,
+  })
+}
 
-    if (!uniqueUsers.size) {
-      return callback(null, labels)
-    }
+async function revertProject(req, res, next) {
+  const { project_id: projectId } = req.params
+  const { version } = req.body
+  const userId = SessionManager.getLoggedInUserId(req.session)
 
-    UserGetter.getUsers(
-      Array.from(uniqueUsers),
-      { first_name: 1, last_name: 1, email: 1 },
-      function (err, rawUsers) {
-        if (err) {
-          return callback(err)
-        }
-        const users = new Map(rawUsers.map(user => [String(user._id), user]))
-
-        labels.forEach(label => {
-          const user = users.get(label.user_id)
-          label.user_display_name = HistoryController._displayNameForUser(user)
-        })
-        callback(null, labels)
-      }
-    )
-  },
+  await RestoreManager.promises.revertProject(userId, projectId, version)
 
-  _displayNameForUser(user) {
-    if (user == null) {
-      return 'Anonymous'
-    }
-    if (user.name) {
-      return user.name
-    }
-    let name = [user.first_name, user.last_name]
-      .filter(n => n != null)
-      .join(' ')
-      .trim()
-    if (name === '') {
-      name = user.email.split('@')[0]
-    }
-    if (!name) {
-      return '?'
+  res.sendStatus(200)
+}
+
+async function getLabels(req, res, next) {
+  const projectId = req.params.Project_id
+
+  let labels = await fetchJson(
+    `${settings.apis.project_history.url}/project/${projectId}/labels`
+  )
+  labels = await _enrichLabels(labels)
+
+  res.json(labels)
+}
+
+async function createLabel(req, res, next) {
+  const projectId = req.params.Project_id
+  const { comment, version } = req.body
+  const userId = SessionManager.getLoggedInUserId(req.session)
+
+  let label = await fetchJson(
+    `${settings.apis.project_history.url}/project/${projectId}/labels`,
+    {
+      method: 'POST',
+      json: { comment, version, user_id: userId },
     }
-    return name
-  },
+  )
+  label = await _enrichLabel(label)
+
+  res.json(label)
+}
 
-  deleteLabel(req, res, next) {
-    const { Project_id: projectId, label_id: labelId } = req.params
-    const userId = SessionManager.getLoggedInUserId(req.session)
+async function _enrichLabel(label) {
+  const newLabel = Object.assign({}, label)
+  if (!label.user_id) {
+    newLabel.user_display_name = _displayNameForUser(null)
+    return newLabel
+  }
 
-    ProjectGetter.getProject(
-      projectId,
-      {
-        owner_ref: true,
-      },
-      (err, project) => {
-        if (err) {
-          return next(err)
-        }
-
-        // If the current user is the project owner, we can use the non-user-specific delete label endpoint.
-        // Otherwise, we have to use the user-specific version (which only deletes the label if it is owned by the user)
-        const deleteEndpointUrl = project.owner_ref.equals(userId)
-          ? `${settings.apis.project_history.url}/project/${projectId}/labels/${labelId}`
-          : `${settings.apis.project_history.url}/project/${projectId}/user/${userId}/labels/${labelId}`
-
-        HistoryController._makeRequest(
-          {
-            method: 'DELETE',
-            url: deleteEndpointUrl,
-          },
-          function (err) {
-            if (err) {
-              return next(err)
-            }
-            res.sendStatus(204)
-          }
-        )
-      }
+  const user = await UserGetter.promises.getUser(label.user_id, {
+    first_name: 1,
+    last_name: 1,
+    email: 1,
+  })
+  newLabel.user_display_name = _displayNameForUser(user)
+  return newLabel
+}
+
+async function _enrichLabels(labels) {
+  if (!labels || !labels.length) {
+    return []
+  }
+  const uniqueUsers = new Set(labels.map(label => label.user_id))
+
+  // For backwards compatibility, and for anonymously created labels in SP
+  // expect missing user_id fields
+  uniqueUsers.delete(undefined)
+
+  if (!uniqueUsers.size) {
+    return labels
+  }
+
+  const rawUsers = await UserGetter.promises.getUsers(Array.from(uniqueUsers), {
+    first_name: 1,
+    last_name: 1,
+    email: 1,
+  })
+  const users = new Map(rawUsers.map(user => [String(user._id), user]))
+
+  labels.forEach(label => {
+    const user = users.get(label.user_id)
+    label.user_display_name = _displayNameForUser(user)
+  })
+  return labels
+}
+
+function _displayNameForUser(user) {
+  if (user == null) {
+    return 'Anonymous'
+  }
+  if (user.name) {
+    return user.name
+  }
+  let name = [user.first_name, user.last_name]
+    .filter(n => n != null)
+    .join(' ')
+    .trim()
+  if (name === '') {
+    name = user.email.split('@')[0]
+  }
+  if (!name) {
+    return '?'
+  }
+  return name
+}
+
+async function deleteLabel(req, res, next) {
+  const { Project_id: projectId, label_id: labelId } = req.params
+  const userId = SessionManager.getLoggedInUserId(req.session)
+
+  const project = await ProjectGetter.promises.getProject(projectId, {
+    owner_ref: true,
+  })
+
+  // If the current user is the project owner, we can use the non-user-specific
+  // delete label endpoint. Otherwise, we have to use the user-specific version
+  // (which only deletes the label if it is owned by the user)
+  const deleteEndpointUrl = project.owner_ref.equals(userId)
+    ? `${settings.apis.project_history.url}/project/${projectId}/labels/${labelId}`
+    : `${settings.apis.project_history.url}/project/${projectId}/user/${userId}/labels/${labelId}`
+
+  await fetchNothing(deleteEndpointUrl, {
+    method: 'DELETE',
+  })
+  res.sendStatus(204)
+}
+
+async function downloadZipOfVersion(req, res, next) {
+  const { project_id: projectId, version } = req.params
+
+  const project = await ProjectDetailsHandler.promises.getDetails(projectId)
+  const v1Id =
+    project.overleaf && project.overleaf.history && project.overleaf.history.id
+
+  if (v1Id == null) {
+    logger.error(
+      { projectId, version },
+      'got request for zip version of non-v1 history project'
     )
-  },
+    return res.sendStatus(402)
+  }
 
-  _makeRequest(options, callback) {
-    return request(options, function (err, response, body) {
-      if (err) {
-        return callback(err)
-      }
-      if (response.statusCode >= 200 && response.statusCode < 300) {
-        callback(null, body)
+  await _pipeHistoryZipToResponse(
+    v1Id,
+    version,
+    `${project.name} (Version ${version})`,
+    req,
+    res
+  )
+}
+
+async function _pipeHistoryZipToResponse(v1ProjectId, version, name, req, res) {
+  if (req.destroyed) {
+    // client has disconnected -- skip project history api call and download
+    return
+  }
+  // increase timeout to 6 minutes
+  res.setTimeout(6 * 60 * 1000)
+  const url = `${settings.apis.v1_history.url}/projects/${v1ProjectId}/version/${version}/zip`
+  const basicAuth = {
+    user: settings.apis.v1_history.user,
+    password: settings.apis.v1_history.pass,
+  }
+
+  if (!Features.hasFeature('saas')) {
+    let stream
+    try {
+      stream = await fetchStream(url, { basicAuth })
+    } catch (err) {
+      if (err instanceof RequestFailedError && err.response.status === 404) {
+        return res.sendStatus(404)
       } else {
-        err = new Error(
-          `history api responded with non-success code: ${response.statusCode}`
-        )
-        callback(err)
+        throw err
       }
-    })
-  },
+    }
 
-  downloadZipOfVersion(req, res, next) {
-    const { project_id: projectId, version } = req.params
-    ProjectDetailsHandler.getDetails(projectId, function (err, project) {
-      if (err) {
-        return next(err)
-      }
-      const v1Id =
-        project.overleaf &&
-        project.overleaf.history &&
-        project.overleaf.history.id
-      if (v1Id == null) {
-        logger.error(
-          { projectId, version },
-          'got request for zip version of non-v1 history project'
-        )
-        return res.sendStatus(402)
+    prepareZipAttachment(res, `${name}.zip`)
+
+    try {
+      await pipeline(stream, res)
+    } catch (err) {
+      // If the downstream request is cancelled, we get an
+      // ERR_STREAM_PREMATURE_CLOSE.
+      if (!isPrematureClose(err)) {
+        throw err
       }
-      HistoryController._pipeHistoryZipToResponse(
-        v1Id,
-        version,
-        `${project.name} (Version ${version})`,
-        req,
-        res,
-        next
-      )
+    }
+    return
+  }
+
+  let body
+  try {
+    body = await fetchJson(url, { method: 'POST', basicAuth })
+  } catch (err) {
+    if (err instanceof RequestFailedError && err.response.status === 404) {
+      throw new Errors.NotFoundError('zip not found')
+    } else {
+      throw err
+    }
+  }
+
+  if (req.destroyed) {
+    // client has disconnected -- skip delayed s3 download
+    return
+  }
+
+  if (!body.zipUrl) {
+    throw new OError('Missing zipUrl, cannot fetch zip file', {
+      v1ProjectId,
+      body,
     })
-  },
+  }
+
+  // retry for about 6 minutes starting with short delay
+  let retryDelay = 2000
+  let attempt = 0
+  while (true) {
+    attempt += 1
+    await setTimeout(retryDelay)
 
-  _pipeHistoryZipToResponse(v1ProjectId, version, name, req, res, next) {
     if (req.destroyed) {
-      // client has disconnected -- skip project history api call and download
+      // client has disconnected -- skip s3 download
       return
     }
-    // increase timeout to 6 minutes
-    res.setTimeout(6 * 60 * 1000)
-    const url = `${settings.apis.v1_history.url}/projects/${v1ProjectId}/version/${version}/zip`
-    const options = {
-      auth: {
-        user: settings.apis.v1_history.user,
-        pass: settings.apis.v1_history.pass,
-      },
-      json: true,
-      url,
-    }
 
-    if (!Features.hasFeature('saas')) {
-      const getReq = request({ ...options, method: 'get' })
-
-      getReq.on('error', function (err) {
-        logger.warn({ err, v1ProjectId, version }, 'history zip download error')
-        res.sendStatus(500)
-      })
-      getReq.on('response', function (response) {
-        const statusCode = response.statusCode
-        if (statusCode !== 200) {
-          logger.warn(
-            { v1ProjectId, version, statusCode },
-            'history zip download failed'
-          )
-          if (statusCode === 404) {
-            res.sendStatus(404)
-          } else {
-            res.sendStatus(500)
-          }
-          return
-        }
-
-        prepareZipAttachment(res, `${name}.zip`)
-        pipeline(response, res, function (err) {
-          // If the downstream request is cancelled, we get an
-          // ERR_STREAM_PREMATURE_CLOSE.
-          if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
-            logger.error({ err, v1ProjectId, version }, 'history API error')
-            next(err)
-          }
-        })
-      })
-      return
+    // increase delay by 1 second up to 10
+    if (retryDelay < 10000) {
+      retryDelay += 1000
     }
 
-    request({ ...options, method: 'post' }, function (err, response, body) {
-      if (err) {
-        OError.tag(err, 'history API error', {
-          v1ProjectId,
-          version,
-        })
-        return next(err)
-      }
-      if (response.statusCode !== 200) {
-        if (response.statusCode === 404) {
-          return next(new Errors.NotFoundError('zip not found'))
-        } else {
-          return next(
-            new OError('Error while getting zip for download', {
-              v1ProjectId,
-              statusCode: response.statusCode,
-            })
-          )
-        }
+    try {
+      const stream = await fetchStream(body.zipUrl)
+      prepareZipAttachment(res, `${name}.zip`)
+      await pipeline(stream, res)
+    } catch (err) {
+      if (attempt > MAX_HISTORY_ZIP_ATTEMPTS) {
+        throw err
       }
-      if (req.destroyed) {
-        // client has disconnected -- skip delayed s3 download
-        return
-      }
-      if (!body.zipUrl) {
-        return next(
-          new OError('Missing zipUrl, cannot fetch zip file', {
-            v1ProjectId,
-            body,
-            statusCode: response.statusCode,
-          })
+
+      if (err instanceof RequestFailedError && err.response.status === 404) {
+        // File not ready yet. Retry.
+        continue
+      } else if (isPrematureClose(err)) {
+        // Downstream request cancelled. Retry.
+        continue
+      } else {
+        // Unknown error. Log and retry.
+        logger.warn(
+          { err, v1ProjectId, version, retryAttempt: attempt },
+          'history s3 proxying error'
         )
+        continue
       }
-      let retryAttempt = 0
-      let retryDelay = 2000
-      // retry for about 6 minutes starting with short delay
-      async.retry(
-        40,
-        callback =>
-          setTimeout(function () {
-            if (req.destroyed) {
-              // client has disconnected -- skip s3 download
-              return callback() // stop async.retry loop
-            }
-
-            // increase delay by 1 second up to 10
-            if (retryDelay < 10000) {
-              retryDelay += 1000
-            }
-            retryAttempt++
-            const getReq = request({
-              url: body.zipUrl,
-              sendImmediately: true,
-            })
-            const abortS3Request = () => getReq.abort()
-            req.on('close', abortS3Request)
-            res.on('timeout', abortS3Request)
-            function cleanupAbortTrigger() {
-              req.off('close', abortS3Request)
-              res.off('timeout', abortS3Request)
-            }
-            getReq.on('response', function (response) {
-              if (response.statusCode !== 200) {
-                cleanupAbortTrigger()
-                return callback(new Error('invalid response'))
-              }
-              // pipe also proxies the headers, but we want to customize these ones
-              delete response.headers['content-disposition']
-              delete response.headers['content-type']
-              res.status(response.statusCode)
-              prepareZipAttachment(res, `${name}.zip`)
-              pipeline(response, res, err => {
-                // If the downstream request is cancelled, we get an
-                // ERR_STREAM_PREMATURE_CLOSE.
-                if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
-                  logger.warn(
-                    { err, v1ProjectId, version, retryAttempt },
-                    'history s3 proxying error'
-                  )
-                }
-              })
-              callback()
-            })
-            getReq.on('error', function (err) {
-              logger.warn(
-                { err, v1ProjectId, version, retryAttempt },
-                'history s3 download error'
-              )
-              cleanupAbortTrigger()
-              callback(err)
-            })
-          }, retryDelay),
-        function (err) {
-          if (err) {
-            OError.tag(err, 'history s3 download failed', {
-              v1ProjectId,
-              version,
-              retryAttempt,
-            })
-            next(err)
-          }
-        }
-      )
-    })
+    }
+
+    // We made it through. No need to retry anymore. Exit loop
+    break
+  }
+}
+
+function isPrematureClose(err) {
+  return (
+    err instanceof Error &&
+    'code' in err &&
+    err.code === 'ERR_STREAM_PREMATURE_CLOSE'
+  )
+}
+
+module.exports = {
+  getBlob: expressify(getBlob),
+  headBlob: expressify(headBlob),
+  proxyToHistoryApi: expressify(proxyToHistoryApi),
+  proxyToHistoryApiAndInjectUserDetails: expressify(
+    proxyToHistoryApiAndInjectUserDetails
+  ),
+  resyncProjectHistory: expressify(resyncProjectHistory),
+  restoreFileFromV2: expressify(restoreFileFromV2),
+  revertFile: expressify(revertFile),
+  revertProject: expressify(revertProject),
+  getLabels: expressify(getLabels),
+  createLabel: expressify(createLabel),
+  deleteLabel: expressify(deleteLabel),
+  downloadZipOfVersion: expressify(downloadZipOfVersion),
+  _displayNameForUser,
+  promises: {
+    _pipeHistoryZipToResponse,
   },
 }

services/web/test/unit/src/History/HistoryControllerTests.js

@@ -1,16 +1,6 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-*/
-// 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 sinon = require('sinon')
 const { expect } = require('chai')
+const { RequestFailedError } = require('@overleaf/fetch-utils')
 
 const Errors = require('../../../../app/src/Features/Errors/Errors')
 
@@ -22,25 +12,59 @@ describe('HistoryController', function () {
   beforeEach(function () {
     this.callback = sinon.stub()
     this.user_id = 'user-id-123'
+    this.project_id = 'mock-project-id'
+    this.stream = sinon.stub()
+    this.fetchResponse = {
+      headers: {
+        get: sinon.stub(),
+      },
+    }
+    this.next = sinon.stub()
+
     this.SessionManager = {
       getLoggedInUserId: sinon.stub().returns(this.user_id),
     }
+
     this.Stream = {
-      pipeline: sinon.stub(),
+      pipeline: sinon.stub().resolves(),
     }
+
+    this.HistoryManager = {
+      promises: {
+        injectUserDetails: sinon.stub(),
+      },
+    }
+
+    this.ProjectEntityUpdateHandler = {
+      promises: {
+        resyncProjectHistory: sinon.stub().resolves(),
+      },
+    }
+
+    this.fetchJson = sinon.stub()
+    this.fetchStream = sinon.stub().resolves(this.stream)
+    this.fetchStreamWithResponse = sinon
+      .stub()
+      .resolves({ stream: this.stream, response: this.fetchResponse })
+    this.fetchNothing = sinon.stub().resolves()
+
     this.HistoryController = SandboxedModule.require(modulePath, {
       requires: {
-        request: (this.request = sinon.stub()),
+        'stream/promises': this.Stream,
         '@overleaf/settings': (this.settings = {}),
-        '@overleaf/fetch-utils': {},
+        '@overleaf/fetch-utils': {
+          fetchJson: this.fetchJson,
+          fetchStream: this.fetchStream,
+          fetchStreamWithResponse: this.fetchStreamWithResponse,
+          fetchNothing: this.fetchNothing,
+        },
         '@overleaf/Metrics': {},
         '../../infrastructure/mongodb': { ObjectId },
-        stream: this.Stream,
         '../Authentication/SessionManager': this.SessionManager,
-        './HistoryManager': (this.HistoryManager = {}),
+        './HistoryManager': this.HistoryManager,
         '../Project/ProjectDetailsHandler': (this.ProjectDetailsHandler = {}),
         '../Project/ProjectEntityUpdateHandler':
-          (this.ProjectEntityUpdateHandler = {}),
+          this.ProjectEntityUpdateHandler,
         '../User/UserGetter': (this.UserGetter = {}),
         '../Project/ProjectGetter': (this.ProjectGetter = {}),
         './RestoreManager': (this.RestoreManager = {}),
@@ -50,172 +74,125 @@ describe('HistoryController', function () {
           .returns(true)),
       },
     })
-    return (this.settings.apis = {
+    this.settings.apis = {
       project_history: {
         url: 'http://project_history.example.com',
       },
-    })
+    }
   })
 
   describe('proxyToHistoryApi', function () {
-    beforeEach(function () {
-      this.req = { url: '/mock/url', method: 'POST' }
-      this.res = 'mock-res'
-      this.next = sinon.stub()
-      this.proxy = sinon.stub()
-      this.request.returns(this.proxy)
+    beforeEach(async function () {
+      this.req = { url: '/mock/url', method: 'POST', session: sinon.stub() }
+      this.res = {
+        set: sinon.stub(),
+      }
+      this.contentType = 'application/json'
+      this.contentLength = 212
+      this.fetchResponse.headers.get
+        .withArgs('Content-Type')
+        .returns(this.contentType)
+      this.fetchResponse.headers.get
+        .withArgs('Content-Length')
+        .returns(this.contentLength)
+      await this.HistoryController.proxyToHistoryApi(
+        this.req,
+        this.res,
+        this.next
+      )
     })
 
-    describe('for a project with the project history flag', function () {
-      beforeEach(function () {
-        this.req.useProjectHistory = true
-        return this.HistoryController.proxyToHistoryApi(
-          this.req,
-          this.res,
-          this.next
-        )
-      })
-
-      it('should get the user id', function () {
-        return this.SessionManager.getLoggedInUserId
-          .calledWith(this.req.session)
-          .should.equal(true)
-      })
-
-      it('should call the project history api', function () {
-        return this.request
-          .calledWith({
-            url: `${this.settings.apis.project_history.url}${this.req.url}`,
-            method: this.req.method,
-            headers: {
-              'X-User-Id': this.user_id,
-            },
-          })
-          .should.equal(true)
-      })
-
-      it('should pipe the response to the client', function () {
-        expect(this.Stream.pipeline).to.have.been.calledWith(
-          this.proxy,
-          this.res
-        )
-      })
+    it('should get the user id', function () {
+      this.SessionManager.getLoggedInUserId.should.have.been.calledWith(
+        this.req.session
+      )
     })
 
-    describe('for a project without the project history flag', function () {
-      beforeEach(function () {
-        this.req.useProjectHistory = false
-        return this.HistoryController.proxyToHistoryApi(
-          this.req,
-          this.res,
-          this.next
-        )
-      })
+    it('should call the project history api', function () {
+      this.fetchStreamWithResponse.should.have.been.calledWith(
+        `${this.settings.apis.project_history.url}${this.req.url}`,
+        {
+          method: this.req.method,
+          headers: {
+            'X-User-Id': this.user_id,
+          },
+        }
+      )
+    })
 
-      it('should get the user id', function () {
-        return this.SessionManager.getLoggedInUserId
-          .calledWith(this.req.session)
-          .should.equal(true)
-      })
+    it('should pipe the response to the client', function () {
+      expect(this.Stream.pipeline).to.have.been.calledWith(
+        this.stream,
+        this.res
+      )
+    })
 
-      it('should pipe the response to the client', function () {
-        expect(this.Stream.pipeline).to.have.been.calledWith(
-          this.proxy,
-          this.res
-        )
-      })
+    it('should propagate the appropriate headers', function () {
+      expect(this.res.set).to.have.been.calledWith(
+        'Content-Type',
+        this.contentType
+      )
+      expect(this.res.set).to.have.been.calledWith(
+        'Content-Length',
+        this.contentLength
+      )
     })
   })
 
   describe('proxyToHistoryApiAndInjectUserDetails', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       this.req = { url: '/mock/url', method: 'POST' }
       this.res = { json: sinon.stub() }
-      this.next = sinon.stub()
-      this.request.yields(null, { statusCode: 200 }, (this.data = 'mock-data'))
-      return (this.HistoryManager.injectUserDetails = sinon
-        .stub()
-        .yields(null, (this.data_with_users = 'mock-injected-data')))
+      this.data = 'mock-data'
+      this.dataWithUsers = 'mock-injected-data'
+      this.fetchJson.resolves(this.data)
+      this.HistoryManager.promises.injectUserDetails.resolves(
+        this.dataWithUsers
+      )
+      await this.HistoryController.proxyToHistoryApiAndInjectUserDetails(
+        this.req,
+        this.res,
+        this.next
+      )
     })
 
-    describe('for a project with the project history flag', function () {
-      beforeEach(function () {
-        this.req.useProjectHistory = true
-        return this.HistoryController.proxyToHistoryApiAndInjectUserDetails(
-          this.req,
-          this.res,
-          this.next
-        )
-      })
-
-      it('should get the user id', function () {
-        return this.SessionManager.getLoggedInUserId
-          .calledWith(this.req.session)
-          .should.equal(true)
-      })
-
-      it('should call the project history api', function () {
-        return this.request
-          .calledWith({
-            url: `${this.settings.apis.project_history.url}${this.req.url}`,
-            method: this.req.method,
-            json: true,
-            headers: {
-              'X-User-Id': this.user_id,
-            },
-          })
-          .should.equal(true)
-      })
-
-      it('should inject the user data', function () {
-        return this.HistoryManager.injectUserDetails
-          .calledWith(this.data)
-          .should.equal(true)
-      })
-
-      it('should return the data with users to the client', function () {
-        return this.res.json.calledWith(this.data_with_users).should.equal(true)
-      })
+    it('should get the user id', function () {
+      this.SessionManager.getLoggedInUserId.should.have.been.calledWith(
+        this.req.session
+      )
     })
 
-    describe('for a project without the project history flag', function () {
-      beforeEach(function () {
-        this.req.useProjectHistory = false
-        return this.HistoryController.proxyToHistoryApiAndInjectUserDetails(
-          this.req,
-          this.res,
-          this.next
-        )
-      })
-
-      it('should get the user id', function () {
-        return this.SessionManager.getLoggedInUserId
-          .calledWith(this.req.session)
-          .should.equal(true)
-      })
+    it('should call the project history api', function () {
+      this.fetchJson.should.have.been.calledWith(
+        `${this.settings.apis.project_history.url}${this.req.url}`,
+        {
+          method: this.req.method,
+          headers: {
+            'X-User-Id': this.user_id,
+          },
+        }
+      )
+    })
 
-      it('should inject the user data', function () {
-        return this.HistoryManager.injectUserDetails
-          .calledWith(this.data)
-          .should.equal(true)
-      })
+    it('should inject the user data', function () {
+      this.HistoryManager.promises.injectUserDetails.should.have.been.calledWith(
+        this.data
+      )
+    })
 
-      it('should return the data with users to the client', function () {
-        return this.res.json.calledWith(this.data_with_users).should.equal(true)
-      })
+    it('should return the data with users to the client', function () {
+      this.res.json.should.have.been.calledWith(this.dataWithUsers)
     })
   })
 
   describe('proxyToHistoryApiAndInjectUserDetails (with the history API failing)', function () {
-    beforeEach(function () {
-      this.req = { url: '/mock/url', method: 'POST', useProjectHistory: true }
+    beforeEach(async function () {
+      this.url = '/mock/url'
+      this.req = { url: this.url, method: 'POST' }
       this.res = { json: sinon.stub() }
-      this.next = sinon.stub()
-      this.request.yields(null, { statusCode: 500 }, (this.data = 'mock-data'))
-      this.HistoryManager.injectUserDetails = sinon
-        .stub()
-        .yields(null, (this.data_with_users = 'mock-injected-data'))
-      return this.HistoryController.proxyToHistoryApiAndInjectUserDetails(
+      this.err = new RequestFailedError(this.url, {}, { status: 500 })
+      this.fetchJson.rejects(this.err)
+      await this.HistoryController.proxyToHistoryApiAndInjectUserDetails(
         this.req,
         this.res,
         this.next
@@ -223,30 +200,30 @@ describe('HistoryController', function () {
     })
 
     it('should not inject the user data', function () {
-      return this.HistoryManager.injectUserDetails
-        .calledWith(this.data)
-        .should.equal(false)
+      this.HistoryManager.promises.injectUserDetails.should.not.have.been.called
     })
 
     it('should not return the data with users to the client', function () {
-      return this.res.json.calledWith(this.data_with_users).should.equal(false)
+      this.res.json.should.not.have.been.called
+    })
+
+    it('should throw an error', function () {
+      this.next.should.have.been.calledWith(this.err)
     })
   })
 
   describe('resyncProjectHistory', function () {
     describe('for a project without project-history enabled', function () {
-      beforeEach(function () {
-        this.project_id = 'mock-project-id'
+      beforeEach(async function () {
         this.req = { params: { Project_id: this.project_id }, body: {} }
         this.res = { setTimeout: sinon.stub(), sendStatus: sinon.stub() }
-        this.next = sinon.stub()
 
         this.error = new Errors.ProjectHistoryDisabledError()
-        this.ProjectEntityUpdateHandler.resyncProjectHistory = sinon
-          .stub()
-          .yields(this.error)
+        this.ProjectEntityUpdateHandler.promises.resyncProjectHistory.rejects(
+          this.error
+        )
 
-        return this.HistoryController.resyncProjectHistory(
+        await this.HistoryController.resyncProjectHistory(
           this.req,
           this.res,
           this.next
@@ -254,22 +231,16 @@ describe('HistoryController', function () {
       })
 
       it('response with a 404', function () {
-        return this.res.sendStatus.calledWith(404).should.equal(true)
+        this.res.sendStatus.should.have.been.calledWith(404)
       })
     })
 
     describe('for a project with project-history enabled', function () {
-      beforeEach(function () {
-        this.project_id = 'mock-project-id'
+      beforeEach(async function () {
         this.req = { params: { Project_id: this.project_id }, body: {} }
         this.res = { setTimeout: sinon.stub(), sendStatus: sinon.stub() }
-        this.next = sinon.stub()
 
-        this.ProjectEntityUpdateHandler.resyncProjectHistory = sinon
-          .stub()
-          .yields()
-
-        return this.HistoryController.resyncProjectHistory(
+        await this.HistoryController.resyncProjectHistory(
           this.req,
           this.res,
           this.next
@@ -281,13 +252,13 @@ describe('HistoryController', function () {
       })
 
       it('resyncs the project', function () {
-        return this.ProjectEntityUpdateHandler.resyncProjectHistory
-          .calledWith(this.project_id)
-          .should.equal(true)
+        this.ProjectEntityUpdateHandler.promises.resyncProjectHistory.should.have.been.calledWith(
+          this.project_id
+        )
       })
 
       it('responds with a 204', function () {
-        return this.res.sendStatus.calledWith(204).should.equal(true)
+        this.res.sendStatus.should.have.been.calledWith(204)
       })
     })
   })