From b9b3cdc9973bf5b46519e805bdc9469be7d5a04d Mon Sep 17 00:00:00 2001
From: Antoine Clausse <antoine.clausse@overleaf.com>
Date: Thu, 15 May 2025 11:01:38 +0200
Subject: [PATCH] Remove returns of promises within functions with callbacks
 (address DeprecationWarning) (#25603)

* [document-updater] Don't return promises within functions with callbacks

Remove the errors:
DeprecationWarning: Calling promisify on a function that returns a Promise is likely a mistake
https://cloudlogging.app.goo.gl/YHDhoarvLEw2w9rXA

* Remove some more unnecessary returns in functions with callbacks, for consistency

* Add `sendCanaryAppliedOp` to excluded methods for promisification

GitOrigin-RevId: fa6d3e47c4e6561dc29d4c15e57c3289fc1f3dfa
---
 libraries/redis-wrapper/RedisLocker.js        | 55 +++++++++----------
 .../app/js/RealTimeRedisManager.js            |  6 +-
 2 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/libraries/redis-wrapper/RedisLocker.js b/libraries/redis-wrapper/RedisLocker.js
index b819ad2188..17ad514246 100644
--- a/libraries/redis-wrapper/RedisLocker.js
+++ b/libraries/redis-wrapper/RedisLocker.js
@@ -97,7 +97,8 @@ module.exports = class RedisLocker {
   }
 
   /**
-   * @param {Callback} callback
+   * @param {string} id
+   * @param {function(Error, boolean, string): void} callback
    */
   tryLock(id, callback) {
     if (callback == null) {
@@ -106,7 +107,7 @@ module.exports = class RedisLocker {
     const lockValue = this.randomLock()
     const key = this.getKey(id)
     const startTime = Date.now()
-    return this.rclient.set(
+    this.rclient.set(
       key,
       lockValue,
       'EX',
@@ -121,7 +122,7 @@ module.exports = class RedisLocker {
           const timeTaken = Date.now() - startTime
           if (timeTaken > MAX_REDIS_REQUEST_LENGTH) {
             // took too long, so try to free the lock
-            return this.releaseLock(id, lockValue, function (err, result) {
+            this.releaseLock(id, lockValue, function (err, result) {
               if (err != null) {
                 return callback(err)
               } // error freeing lock
@@ -139,7 +140,8 @@ module.exports = class RedisLocker {
   }
 
   /**
-   * @param {Callback} callback
+   * @param {string} id
+   * @param {function(Error, string): void} callback
    */
   getLock(id, callback) {
     if (callback == null) {
@@ -153,7 +155,7 @@ module.exports = class RedisLocker {
         return callback(e)
       }
 
-      return this.tryLock(id, (error, gotLock, lockValue) => {
+      this.tryLock(id, (error, gotLock, lockValue) => {
         if (error != null) {
           return callback(error)
         }
@@ -173,14 +175,15 @@ module.exports = class RedisLocker {
   }
 
   /**
-   * @param {Callback} callback
+   * @param {string} id
+   * @param {function(Error, boolean): void} callback
    */
   checkLock(id, callback) {
     if (callback == null) {
       callback = function () {}
     }
     const key = this.getKey(id)
-    return this.rclient.exists(key, (err, exists) => {
+    this.rclient.exists(key, (err, exists) => {
       if (err != null) {
         return callback(err)
       }
@@ -196,30 +199,26 @@ module.exports = class RedisLocker {
   }
 
   /**
-   * @param {Callback} callback
+   * @param {string} id
+   * @param {string} lockValue
+   * @param {function(Error, boolean): void} callback
    */
   releaseLock(id, lockValue, callback) {
     const key = this.getKey(id)
-    return this.rclient.eval(
-      UNLOCK_SCRIPT,
-      1,
-      key,
-      lockValue,
-      (err, result) => {
-        if (err != null) {
-          return callback(err)
-        } else if (result != null && result !== 1) {
-          // successful unlock should release exactly one key
-          logger.error(
-            { id, key, lockValue, redis_err: err, redis_result: result },
-            'unlocking error'
-          )
-          metrics.inc(this.metricsPrefix + '-unlock-error')
-          return callback(new Error('tried to release timed out lock'))
-        } else {
-          return callback(null, result)
-        }
+    this.rclient.eval(UNLOCK_SCRIPT, 1, key, lockValue, (err, result) => {
+      if (err != null) {
+        return callback(err)
+      } else if (result != null && result !== 1) {
+        // successful unlock should release exactly one key
+        logger.error(
+          { id, key, lockValue, redis_err: err, redis_result: result },
+          'unlocking error'
+        )
+        metrics.inc(this.metricsPrefix + '-unlock-error')
+        return callback(new Error('tried to release timed out lock'))
+      } else {
+        return callback(null, result)
       }
-    )
+    })
   }
 }
diff --git a/services/document-updater/app/js/RealTimeRedisManager.js b/services/document-updater/app/js/RealTimeRedisManager.js
index 08bf132dec..2b67971c5c 100644
--- a/services/document-updater/app/js/RealTimeRedisManager.js
+++ b/services/document-updater/app/js/RealTimeRedisManager.js
@@ -49,7 +49,7 @@ const RealTimeRedisManager = {
       MAX_OPS_PER_ITERATION,
       -1
     )
-    return multi.exec(function (error, replys) {
+    multi.exec(function (error, replys) {
       if (error != null) {
         return callback(error)
       }
@@ -80,7 +80,7 @@ const RealTimeRedisManager = {
   },
 
   getUpdatesLength(docId, callback) {
-    return rclient.llen(Keys.pendingUpdates({ doc_id: docId }), callback)
+    rclient.llen(Keys.pendingUpdates({ doc_id: docId }), callback)
   },
 
   sendCanaryAppliedOp({ projectId, docId, op }) {
@@ -132,5 +132,5 @@ const RealTimeRedisManager = {
 
 module.exports = RealTimeRedisManager
 module.exports.promises = promisifyAll(RealTimeRedisManager, {
-  without: ['sendData'],
+  without: ['sendCanaryAppliedOp', 'sendData'],
 })