Merge pull request #1902 from overleaf/msm-return-429-tdps-update-too-many-connections

Project on cooldown returns 429 on TDPS update

GitOrigin-RevId: b8da35a61be7508bc56d7798d233c186f694c364

services/web/app/src/Features/ThirdPartyDataStore/TpdsController.js

@@ -42,11 +42,19 @@ module.exports = {
           'sending response that tpdsUpdate has been completed'
         )
         if (err != null) {
-          logger.err(
-            { err, user_id, filePath },
-            'error reciving update from tpds'
-          )
-          return res.sendStatus(500)
+          if (err.name === 'TooManyRequestsError') {
+            logger.warn(
+              { err, user_id, filePath },
+              'tpds update failed to be processed, too many requests'
+            )
+            return res.sendStatus(429)
+          } else {
+            logger.err(
+              { err, user_id, filePath },
+              'error reciving update from tpds'
+            )
+            return res.sendStatus(500)
+          }
         } else {
           logger.log(
             { user_id, filePath, projectName },

services/web/test/unit/src/ThirdPartyDataStore/TpdsControllerTests.js

@@ -12,6 +12,7 @@
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 require('chai').should()
+const Errors = require('../../../../app/src/Features/Errors/Errors')
 const modulePath = require('path').join(
   __dirname,
   '../../../../app/src/Features/ThirdPartyDataStore/TpdsController.js'
@@ -26,6 +27,7 @@ describe('TpdsController', function() {
         './UpdateMerger': (this.UpdateMerger = {}),
         'logger-sharelatex': {
           log() {},
+          warn() {},
           err() {}
         },
         'metrics-sharelatex': {
@@ -34,11 +36,11 @@ describe('TpdsController', function() {
       }
     })
 
-    return (this.user_id = 'dsad29jlkjas')
+    this.user_id = 'dsad29jlkjas'
   })
 
-  describe('getting an update', () =>
-    it('should process the update with the update reciver', function(done) {
+  describe('getting an update', () => {
+    it('should process the update with the update receiver', function(done) {
       const path = '/projectName/here.txt'
       const req = {
         pause() {},
@@ -62,11 +64,56 @@ describe('TpdsController', function() {
               this.source
             )
             .should.equal(true)
-          return done()
+          done()
         }
       }
-      return this.TpdsController.mergeUpdate(req, res)
-    }))
+      this.TpdsController.mergeUpdate(req, res)
+    })
+
+    it('should return a 500 error when the update receiver fails', function() {
+      const path = '/projectName/here.txt'
+      const req = {
+        pause() {},
+        params: { 0: path, user_id: this.user_id },
+        session: {
+          destroy() {}
+        },
+        headers: {
+          'x-sl-update-source': (this.source = 'dropbox')
+        }
+      }
+      this.TpdsUpdateHandler.newUpdate = sinon
+        .stub()
+        .callsArgWith(5, 'update-receiver-error')
+      const res = {
+        sendStatus: sinon.stub()
+      }
+      this.TpdsController.mergeUpdate(req, res)
+      res.sendStatus.calledWith(500).should.equal(true)
+    })
+
+    it('should return a 429 error when the update receiver fails due to too many requests error', function() {
+      const path = '/projectName/here.txt'
+      const req = {
+        pause() {},
+        params: { 0: path, user_id: this.user_id },
+        session: {
+          destroy() {}
+        },
+        headers: {
+          'x-sl-update-source': (this.source = 'dropbox')
+        }
+      }
+      this.TpdsUpdateHandler.newUpdate = sinon
+        .stub()
+        .callsArgWith(5, new Errors.TooManyRequestsError('project on cooldown'))
+      const res = {
+        sendStatus: sinon.stub()
+      }
+      this.TpdsController.mergeUpdate(req, res)
+      res.sendStatus.calledWith(429).should.equal(true)
+    })
+  })
 
   describe('getting a delete update', () =>
     it('should process the delete with the update reciver', function(done) {
@@ -86,10 +133,10 @@ describe('TpdsController', function() {
           this.TpdsUpdateHandler.deleteUpdate
             .calledWith(this.user_id, 'projectName', '/here.txt', this.source)
             .should.equal(true)
-          return done()
+          done()
         }
       }
-      return this.TpdsController.deleteUpdate(req, res)
+      this.TpdsController.deleteUpdate(req, res)
     }))
 
   describe('parseParams', function() {
@@ -99,16 +146,16 @@ describe('TpdsController', function() {
       const result = this.TpdsController.parseParams(req)
       result.user_id.should.equal(this.user_id)
       result.filePath.should.equal('/')
-      return result.projectName.should.equal(path)
+      result.projectName.should.equal(path)
     })
 
-    it('should take the project name off the start and it with no slashes in', function() {
+    it('should take the project name off the start and return it with no slashes in', function() {
       const path = '/project/file.tex'
       const req = { params: { 0: path, user_id: this.user_id } }
       const result = this.TpdsController.parseParams(req)
       result.user_id.should.equal(this.user_id)
       result.filePath.should.equal('/file.tex')
-      return result.projectName.should.equal('project')
+      result.projectName.should.equal('project')
     })
 
     it('should take the project name of and return a slash for the file path', function() {
@@ -116,7 +163,7 @@ describe('TpdsController', function() {
       const req = { params: { 0: path, user_id: this.user_id } }
       const result = this.TpdsController.parseParams(req)
       result.projectName.should.equal('project_name')
-      return result.filePath.should.equal('/')
+      result.filePath.should.equal('/')
     })
   })
 
@@ -137,11 +184,11 @@ describe('TpdsController', function() {
       }
       this.res = { sendStatus: sinon.stub() }
 
-      return this.TpdsController.updateProjectContents(this.req, this.res)
+      this.TpdsController.updateProjectContents(this.req, this.res)
     })
 
     it('should merge the update', function() {
-      return this.UpdateMerger.mergeUpdate
+      this.UpdateMerger.mergeUpdate
         .calledWith(
           null,
           this.project_id,
@@ -153,7 +200,7 @@ describe('TpdsController', function() {
     })
 
     it('should return a success', function() {
-      return this.res.sendStatus.calledWith(200).should.equal(true)
+      this.res.sendStatus.calledWith(200).should.equal(true)
     })
   })
 
@@ -174,17 +221,17 @@ describe('TpdsController', function() {
       }
       this.res = { sendStatus: sinon.stub() }
 
-      return this.TpdsController.deleteProjectContents(this.req, this.res)
+      this.TpdsController.deleteProjectContents(this.req, this.res)
     })
 
     it('should delete the file', function() {
-      return this.UpdateMerger.deleteUpdate
+      this.UpdateMerger.deleteUpdate
         .calledWith(null, this.project_id, `/${this.path}`, this.source)
         .should.equal(true)
     })
 
     it('should return a success', function() {
-      return this.res.sendStatus.calledWith(200).should.equal(true)
+      this.res.sendStatus.calledWith(200).should.equal(true)
     })
   })
 })