Convert to async/await

GitOrigin-RevId: 45c139a64c848650f1f51e64e8ebd233211241fb

package-lock.json

@@ -44163,6 +44163,7 @@
       "name": "@overleaf/notifications",
       "license": "ISC",
       "dependencies": {
+        "@overleaf/fetch-utils": "*",
         "@overleaf/logger": "*",
         "@overleaf/metrics": "*",
         "@overleaf/mongo-utils": "*",
@@ -44173,11 +44174,9 @@
         "bunyan": "^1.8.15",
         "express": "^4.21.2",
         "method-override": "^3.0.0",
-        "mongodb-legacy": "6.1.3",
-        "request": "^2.88.2"
+        "mongodb-legacy": "6.1.3"
       },
       "devDependencies": {
-        "@types/request": "^2.48.12",
         "chai-as-promised": "^7.1.1",
         "typescript": "^5.0.4",
         "vitest": "^3.2.4"

services/notifications/app.js

@@ -38,16 +38,7 @@ app.delete(
 
 app.get('/status', (req, res) => res.send('notifications is up'))
 
-app.get('/health_check', (req, res) =>
-  HealthCheckController.check(function (err) {
-    if (err) {
-      logger.err({ err }, 'error performing health check')
-      res.sendStatus(500)
-    } else {
-      res.sendStatus(200)
-    }
-  })
-)
+app.get('/health_check', HealthCheckController.check)
 
 app.get('*', (req, res) => res.sendStatus(404))
 

services/notifications/app/js/HealthCheckController.js

@@ -1,110 +1,130 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 import { db, ObjectId } from './mongodb.js'
-import request from 'request'
-import async from 'async'
 import settings from '@overleaf/settings'
 import logger from '@overleaf/logger'
+import {
+  fetchJson,
+  fetchNothing,
+  RequestFailedError,
+} from '@overleaf/fetch-utils'
+import { expressify } from '@overleaf/promise-utils'
 
 const { port } = settings.internal.notifications
 
-export default {
-  check(callback) {
-    const userId = new ObjectId()
-    const cleanupNotifications = callback =>
-      db.notifications.deleteOne({ user_id: userId }, callback)
+function makeUrl(userId, endPath = '') {
+  return new URL(`/user/${userId}/${endPath}`, `http://127.0.0.1:${port}`)
+}
+
+async function makeNotification(notificationKey, userId) {
+  const postOpts = {
+    method: 'POST',
+    json: {
+      key: notificationKey,
+      messageOpts: '',
+      templateKey: 'f4g5',
+      user_id: userId,
+    },
+    signal: AbortSignal.timeout(5000),
+  }
+  const url = makeUrl(userId)
+  await fetchNothing(url, postOpts)
+}
 
-    let notificationKey = `smoke-test-notification-${new ObjectId()}`
-    const getOpts = endPath => ({
-      url: `http://127.0.0.1:${port}/user/${userId}${endPath}`,
-      timeout: 5000,
+async function getUsersNotifications(userId) {
+  const url = makeUrl(userId)
+  try {
+    return await fetchJson(url, {
+      signal: AbortSignal.timeout(5000),
     })
-    logger.debug(
-      { opts: getOpts(), key: notificationKey, userId },
-      'Health Check: running'
+  } catch (err) {
+    if (err instanceof RequestFailedError) {
+      logger.err({ err }, 'Non-2xx status code received')
+      throw err
+    }
+    logger.err({ err }, 'Health Check: error getting notification')
+    throw err
+  }
+}
+
+async function userHasNotification(userId, notificationKey) {
+  const body = await getUsersNotifications(userId)
+  const hasNotification = body.some(
+    notification =>
+      notification.key === notificationKey && notification.user_id === userId
+  )
+  if (hasNotification) {
+    return body
+  } else {
+    logger.err(
+      { body, notificationKey },
+      'Health Check: notification not in response'
     )
-    const jobs = [
-      function (cb) {
-        const opts = getOpts('/')
-        opts.json = {
-          key: notificationKey,
-          messageOpts: '',
-          templateKey: 'f4g5',
-          user_id: userId,
-        }
-        return request.post(opts, cb)
-      },
-      function (cb) {
-        const opts = getOpts('/')
-        opts.json = true
-        return request.get(opts, function (err, res, body) {
-          if (err != null) {
-            logger.err({ err }, 'Health Check: error getting notification')
-            return callback(err)
-          } else if (res.statusCode !== 200) {
-            const e = `status code not 200 ${res.statusCode}`
-            logger.err({ err }, e)
-            return cb(e)
-          }
-          const hasNotification = body.some(
-            notification =>
-              notification.key === notificationKey &&
-              notification.user_id === userId.toString()
-          )
-          if (hasNotification) {
-            return cb(null, body)
-          } else {
-            logger.err(
-              { body, notificationKey },
-              'Health Check: notification not in response'
-            )
-            return cb(new Error('notification not found in response'))
-          }
-        })
+    throw new Error('notification not found in response')
+  }
+}
+
+async function cleanupNotifications(userId) {
+  await db.notifications.deleteOne({ user_id: userId })
+}
+
+async function deleteNotification(userId, notificationId, notificationKey) {
+  const deleteByIdUrl = makeUrl(userId, `notification/${notificationId}`)
+  try {
+    await fetchNothing(deleteByIdUrl, {
+      signal: AbortSignal.timeout(5000),
+      method: 'DELETE',
+    })
+  } catch (err) {
+    logger.err(
+      { err, url: deleteByIdUrl },
+      'Health Check: error cleaning up notification'
+    )
+    throw err
+  }
+
+  const deleteByKeyUrl = makeUrl(userId)
+
+  try {
+    await fetchNothing(deleteByKeyUrl, {
+      signal: AbortSignal.timeout(5000),
+      method: 'DELETE',
+      body: {
+        key: notificationKey,
       },
-    ]
-    return async.series(jobs, function (err, body) {
-      if (err != null) {
-        logger.err({ err }, 'Health Check: error running health check')
-        return cleanupNotifications(() => callback(err))
-      } else {
-        const notificationId = body[1][0]._id
-        notificationKey = body[1][0].key
-        let opts = getOpts(`/notification/${notificationId}`)
-        logger.debug(
-          { notificationId, notificationKey },
-          'Health Check: doing cleanup'
-        )
-        return request.del(opts, function (err, res, body) {
-          if (err != null) {
-            logger.err(
-              err,
-              opts,
-              'Health Check: error cleaning up notification'
-            )
-            return callback(err)
-          }
-          opts = getOpts('')
-          opts.json = { key: notificationKey }
-          return request.del(opts, function (err, res, body) {
-            if (err != null) {
-              logger.err(
-                err,
-                opts,
-                'Health Check: error cleaning up notification'
-              )
-              return callback(err)
-            }
-            return cleanupNotifications(callback)
-          })
-        })
-      }
     })
-  },
+  } catch (err) {
+    logger.err(
+      { err, url: deleteByKeyUrl },
+      'Health Check: error cleaning up notification'
+    )
+    throw err
+  }
+}
+
+async function check(req, res) {
+  const userId = new ObjectId().toString()
+  let notificationKey = `smoke-test-notification-${new ObjectId()}`
+
+  logger.debug({ userId, key: notificationKey }, 'Health Check: running')
+
+  await makeNotification(notificationKey, userId)
+  try {
+    const body = await userHasNotification(userId, notificationKey)
+    const notificationId = body[0]._id
+    notificationKey = body[0].key
+    logger.debug(
+      { notificationId, notificationKey },
+      'Health Check: doing cleanup'
+    )
+    await deleteNotification(userId, notificationId, notificationKey)
+    res.sendStatus(200)
+  } catch (err) {
+    logger.err({ err }, 'Health Check: error running health check')
+    res.sendStatus(500)
+  } finally {
+    await cleanupNotifications(userId)
+  }
+}
+
+export default {
+  check: expressify(check),
 }

services/notifications/app/js/Notifications.js

@@ -1,121 +1,109 @@
 import logger from '@overleaf/logger'
 import { db, ObjectId } from './mongodb.js'
 
-export default {
-  getUserNotifications(userId, callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    const query = {
-      user_id: new ObjectId(userId),
-      templateKey: { $exists: true },
-    }
-    db.notifications.find(query).toArray(callback)
-  },
+async function getUserNotifications(userId) {
+  const query = {
+    user_id: new ObjectId(userId),
+    templateKey: { $exists: true },
+  }
+  return await db.notifications.find(query).toArray()
+}
 
-  _countExistingNotifications(userId, notification, callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    const query = {
-      user_id: new ObjectId(userId),
-      key: notification.key,
+async function _countExistingNotifications(userId, notification) {
+  const query = {
+    user_id: new ObjectId(userId),
+    key: notification.key,
+  }
+  return await db.notifications.count(query)
+}
+
+async function addNotification(userId, notification, callback) {
+  const count = await _countExistingNotifications(userId, notification)
+  if (count !== 0 && !notification.forceCreate) {
+    return
+  }
+  const doc = {
+    user_id: new ObjectId(userId),
+    key: notification.key,
+    messageOpts: notification.messageOpts,
+    templateKey: notification.templateKey,
+  }
+  // TTL index on the optional `expires` field, which should arrive as an iso date-string, corresponding to
+  // a datetime in the future when the document should be automatically removed.
+  // in Mongo, TTL indexes only work on date fields, and ignore the document when that field is missing
+  // see `README.md` for instruction on creating TTL index
+  if (notification.expires != null) {
+    try {
+      doc.expires = new Date(notification.expires)
+      // _testValue assignment will throw if `expires` is not a valid date
+      // eslint-disable-next-line no-unused-vars
+      const _testValue = doc.expires.toISOString()
+    } catch (error) {
+      logger.error(
+        { userId, expires: notification.expires },
+        'error converting `expires` field to Date'
+      )
+      throw error
     }
-    return db.notifications.count(query, function (err, count) {
-      if (err != null) {
-        return callback(err)
-      }
-      return callback(null, count)
-    })
-  },
+  }
+  return await db.notifications.updateOne(
+    { user_id: doc.user_id, key: notification.key },
+    { $set: doc },
+    { upsert: true }
+  )
+}
 
-  addNotification(userId, notification, callback) {
-    return this._countExistingNotifications(
-      userId,
-      notification,
-      function (err, count) {
-        if (err != null) {
-          return callback(err)
-        }
-        if (count !== 0 && !notification.forceCreate) {
-          return callback()
-        }
-        const doc = {
-          user_id: new ObjectId(userId),
-          key: notification.key,
-          messageOpts: notification.messageOpts,
-          templateKey: notification.templateKey,
-        }
-        // TTL index on the optional `expires` field, which should arrive as an iso date-string, corresponding to
-        // a datetime in the future when the document should be automatically removed.
-        // in Mongo, TTL indexes only work on date fields, and ignore the document when that field is missing
-        // see `README.md` for instruction on creating TTL index
-        if (notification.expires != null) {
-          try {
-            doc.expires = new Date(notification.expires)
-            const _testValue = doc.expires.toISOString()
-          } catch (error) {
-            err = error
-            logger.error(
-              { userId, expires: notification.expires },
-              'error converting `expires` field to Date'
-            )
-            return callback(err)
-          }
-        }
-        db.notifications.updateOne(
-          { user_id: doc.user_id, key: notification.key },
-          { $set: doc },
-          { upsert: true },
-          callback
-        )
-      }
-    )
-  },
+async function removeNotificationId(userId, notificationId) {
+  const searchOps = {
+    user_id: new ObjectId(userId),
+    _id: new ObjectId(notificationId),
+  }
+  const updateOperation = { $unset: { templateKey: true, messageOpts: true } }
+  return await db.notifications.updateOne(searchOps, updateOperation)
+}
 
-  removeNotificationId(userId, notificationId, callback) {
-    const searchOps = {
-      user_id: new ObjectId(userId),
-      _id: new ObjectId(notificationId),
-    }
-    const updateOperation = { $unset: { templateKey: true, messageOpts: true } }
-    db.notifications.updateOne(searchOps, updateOperation, callback)
-  },
+async function removeNotificationKey(userId, notificationKey) {
+  const searchOps = {
+    user_id: new ObjectId(userId),
+    key: notificationKey,
+  }
+  const updateOperation = { $unset: { templateKey: true } }
+  return await db.notifications.updateOne(searchOps, updateOperation)
+}
 
-  removeNotificationKey(userId, notificationKey, callback) {
-    const searchOps = {
-      user_id: new ObjectId(userId),
-      key: notificationKey,
-    }
-    const updateOperation = { $unset: { templateKey: true } }
-    db.notifications.updateOne(searchOps, updateOperation, callback)
-  },
+async function removeNotificationByKeyOnly(notificationKey) {
+  const searchOps = { key: notificationKey }
+  const updateOperation = { $unset: { templateKey: true } }
+  return await db.notifications.updateOne(searchOps, updateOperation)
+}
 
-  removeNotificationByKeyOnly(notificationKey, callback) {
-    const searchOps = { key: notificationKey }
-    const updateOperation = { $unset: { templateKey: true } }
-    db.notifications.updateOne(searchOps, updateOperation, callback)
-  },
+async function countNotificationsByKeyOnly(notificationKey) {
+  const searchOps = { key: notificationKey, templateKey: { $exists: true } }
+  return await db.notifications.countDocuments(searchOps)
+}
 
-  countNotificationsByKeyOnly(notificationKey, callback) {
-    const searchOps = { key: notificationKey, templateKey: { $exists: true } }
-    db.notifications.count(searchOps, callback)
-  },
+async function deleteUnreadNotificationsByKeyOnlyBulk(notificationKey) {
+  if (typeof notificationKey !== 'string') {
+    throw new Error('refusing to bulk delete arbitrary notifications')
+  }
+  const searchOps = { key: notificationKey, templateKey: { $exists: true } }
+  const result = await db.notifications.deleteMany(searchOps)
+  return result.deletedCount
+}
 
-  deleteUnreadNotificationsByKeyOnlyBulk(notificationKey, callback) {
-    if (typeof notificationKey !== 'string') {
-      throw new Error('refusing to bulk delete arbitrary notifications')
-    }
-    const searchOps = { key: notificationKey, templateKey: { $exists: true } }
-    db.notifications.deleteMany(searchOps, (err, result) => {
-      if (err) return callback(err)
-      callback(null, result.deletedCount)
-    })
-  },
+// hard delete of doc, rather than removing the templateKey
+async function deleteNotificationByKeyOnly(notificationKey) {
+  const searchOps = { key: notificationKey }
+  return await db.notifications.deleteOne(searchOps)
+}
 
-  // hard delete of doc, rather than removing the templateKey
-  deleteNotificationByKeyOnly(notificationKey, callback) {
-    const searchOps = { key: notificationKey }
-    db.notifications.deleteOne(searchOps, callback)
-  },
+export default {
+  addNotification,
+  getUserNotifications,
+  removeNotificationId,
+  removeNotificationKey,
+  removeNotificationByKeyOnly,
+  countNotificationsByKeyOnly,
+  deleteUnreadNotificationsByKeyOnlyBulk,
+  deleteNotificationByKeyOnly,
 }

services/notifications/app/js/NotificationsController.js

@@ -1,117 +1,103 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 import logger from '@overleaf/logger'
 import metrics from '@overleaf/metrics'
 import Notifications from './Notifications.js'
+import { expressify } from '@overleaf/promise-utils'
 
-export default {
-  getUserNotifications(req, res, next) {
-    logger.debug(
-      { userId: req.params.user_id },
-      'getting user unread notifications'
-    )
-    metrics.inc('getUserNotifications')
-    return Notifications.getUserNotifications(
-      req.params.user_id,
-      (err, notifications) => {
-        if (err) return next(err)
-        res.json(notifications)
-      }
-    )
-  },
+async function getUserNotifications(req, res, next) {
+  logger.debug(
+    { userId: req.params.user_id },
+    'getting user unread notifications'
+  )
+  metrics.inc('getUserNotifications')
+  const notifications = await Notifications.getUserNotifications(
+    req.params.user_id
+  )
+  res.json(notifications)
+}
 
-  addNotification(req, res) {
-    logger.debug(
-      { userId: req.params.user_id, notification: req.body },
-      'adding notification'
-    )
-    metrics.inc('addNotification')
-    return Notifications.addNotification(
-      req.params.user_id,
-      req.body,
-      function (err, notifications) {
-        if (err != null) {
-          return res.sendStatus(500)
-        } else {
-          return res.sendStatus(200)
-        }
-      }
-    )
-  },
+async function addNotification(req, res) {
+  logger.debug(
+    { userId: req.params.user_id, notification: req.body },
+    'adding notification'
+  )
+  metrics.inc('addNotification')
+  try {
+    await Notifications.addNotification(req.params.user_id, req.body)
+    res.sendStatus(200)
+  } catch (err) {
+    res.sendStatus(500)
+  }
+}
 
-  removeNotificationId(req, res, next) {
-    logger.debug(
-      {
-        userId: req.params.user_id,
-        notificationId: req.params.notification_id,
-      },
-      'mark id notification as read'
-    )
-    metrics.inc('removeNotificationId')
-    return Notifications.removeNotificationId(
-      req.params.user_id,
-      req.params.notification_id,
-      err => {
-        if (err) return next(err)
-        res.sendStatus(200)
-      }
-    )
-  },
+async function removeNotificationId(req, res) {
+  logger.debug(
+    {
+      userId: req.params.user_id,
+      notificationId: req.params.notification_id,
+    },
+    'mark id notification as read'
+  )
+  metrics.inc('removeNotificationId')
+  await Notifications.removeNotificationId(
+    req.params.user_id,
+    req.params.notification_id
+  )
+  res.sendStatus(200)
+}
 
-  removeNotificationKey(req, res, next) {
-    logger.debug(
-      { userId: req.params.user_id, notificationKey: req.body.key },
-      'mark key notification as read'
-    )
-    metrics.inc('removeNotificationKey')
-    return Notifications.removeNotificationKey(
-      req.params.user_id,
-      req.body.key,
-      (err, notifications) => {
-        if (err) return next(err)
-        res.sendStatus(200)
-      }
-    )
-  },
+async function removeNotificationKey(req, res) {
+  logger.debug(
+    { userId: req.params.user_id, notificationKey: req.body.key },
+    'mark key notification as read'
+  )
+  metrics.inc('removeNotificationKey')
+  await Notifications.removeNotificationKey(req.params.user_id, req.body.key)
 
-  removeNotificationByKeyOnly(req, res, next) {
-    const notificationKey = req.params.key
-    logger.debug({ notificationKey }, 'mark notification as read by key only')
-    metrics.inc('removeNotificationKey')
-    return Notifications.removeNotificationByKeyOnly(notificationKey, err => {
-      if (err) return next(err)
-      res.sendStatus(200)
-    })
-  },
+  res.sendStatus(200)
+}
 
-  countNotificationsByKeyOnly(req, res) {
-    const notificationKey = req.params.key
-    Notifications.countNotificationsByKeyOnly(notificationKey, (err, count) => {
-      if (err) {
-        logger.err({ err, notificationKey }, 'cannot count by key')
-        return res.sendStatus(500)
-      }
-      res.json({ count })
-    })
-  },
+async function removeNotificationByKeyOnly(req, res) {
+  const notificationKey = req.params.key
+  logger.debug({ notificationKey }, 'mark notification as read by key only')
+  metrics.inc('removeNotificationKey')
+  await Notifications.removeNotificationByKeyOnly(notificationKey)
+  res.sendStatus(200)
+}
 
-  deleteUnreadNotificationsByKeyOnlyBulk(req, res) {
-    const notificationKey = req.params.key
-    Notifications.deleteUnreadNotificationsByKeyOnlyBulk(
-      notificationKey,
-      (err, count) => {
-        if (err) {
-          logger.err({ err, notificationKey }, 'cannot bulk remove by key')
-          return res.sendStatus(500)
-        }
-        res.json({ count })
-      }
-    )
-  },
+async function countNotificationsByKeyOnly(req, res) {
+  const notificationKey = req.params.key
+  try {
+    const count =
+      await Notifications.countNotificationsByKeyOnly(notificationKey)
+    res.json({ count })
+  } catch (err) {
+    logger.err({ err, notificationKey }, 'cannot count by key')
+    res.sendStatus(500)
+  }
+}
+
+async function deleteUnreadNotificationsByKeyOnlyBulk(req, res) {
+  const notificationKey = req.params.key
+  try {
+    const count =
+      await Notifications.deleteUnreadNotificationsByKeyOnlyBulk(
+        notificationKey
+      )
+    res.json({ count })
+  } catch (err) {
+    logger.err({ err, notificationKey }, 'cannot bulk remove by key')
+    res.sendStatus(500)
+  }
+}
+
+export default {
+  getUserNotifications: expressify(getUserNotifications),
+  addNotification: expressify(addNotification),
+  deleteUnreadNotificationsByKeyOnlyBulk: expressify(
+    deleteUnreadNotificationsByKeyOnlyBulk
+  ),
+  removeNotificationByKeyOnly: expressify(removeNotificationByKeyOnly),
+  removeNotificationId: expressify(removeNotificationId),
+  removeNotificationKey: expressify(removeNotificationKey),
+  countNotificationsByKeyOnly: expressify(countNotificationsByKeyOnly),
 }

services/notifications/package.json

@@ -19,6 +19,7 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
+    "@overleaf/fetch-utils": "*",
     "@overleaf/logger": "*",
     "@overleaf/metrics": "*",
     "@overleaf/mongo-utils": "*",
@@ -29,11 +30,9 @@
     "bunyan": "^1.8.15",
     "express": "^4.21.2",
     "method-override": "^3.0.0",
-    "mongodb-legacy": "6.1.3",
-    "request": "^2.88.2"
+    "mongodb-legacy": "6.1.3"
   },
   "devDependencies": {
-    "@types/request": "^2.48.12",
     "chai-as-promised": "^7.1.1",
     "typescript": "^5.0.4",
     "vitest": "^3.2.4"

services/notifications/test/unit/js/Notifications.test.js

@@ -38,7 +38,7 @@ describe('Notifications Tests', () => {
       ObjectId,
     }))
 
-    notifications = await import(modulePath)
+    notifications = (await import(modulePath)).default
 
     stubbedNotification = {
       user_id: new ObjectId(userId),

services/notifications/test/unit/js/NotificationsController.test.js

@@ -23,7 +23,9 @@ describe('Notifications Controller', () => {
       removeNotificationKey: vi.fn(),
     }
 
-    vi.doMock('../../../app/js/Notifications', () => notifications)
+    vi.doMock('../../../app/js/Notifications', () => ({
+      default: notifications,
+    }))
 
     vi.doMock('@overleaf/metrics', () => ({
       default: {