Browse Source

Merge pull request #78 from overleaf/spd-web-ratelimit

Generate retryable error when hitting rate limits in web
Simon Detheridge 7 years ago
parent
commit
b6a7a0ab4c

+ 10 - 0
services/real-time/app/coffee/Errors.coffee

@@ -0,0 +1,10 @@
+CodedError = (message, code) ->
+	error = new Error(message)
+	error.name = "CodedError"
+	error.code = code
+	error.__proto__ = CodedError.prototype
+	return error
+CodedError.prototype.__proto__ = Error.prototype
+
+module.exports = Errors =
+	CodedError: CodedError

+ 3 - 0
services/real-time/app/coffee/Router.coffee

@@ -21,6 +21,9 @@ module.exports = Router =
 				attrs[key] = value
 			attrs.client_id = client.id
 			attrs.err = error
+			if error.name == "CodedError"
+				logger.warn attrs, error.message, code: error.code
+				return callback {message: error.message, code: error.code}
 			if error.message in ["not authorized", "doc updater could not load requested ops", "no project_id found on client"]
 				logger.warn attrs, error.message
 				return callback {message: error.message}

+ 5 - 1
services/real-time/app/coffee/WebApiManager.coffee

@@ -1,6 +1,7 @@
 request = require "request"
 settings = require "settings-sharelatex"
 logger = require "logger-sharelatex"
+{ CodedError } = require "./Errors"
 
 module.exports = WebApiManager =
 	joinProject: (project_id, user, callback = (error, project, privilegeLevel) ->) ->
@@ -24,7 +25,10 @@ module.exports = WebApiManager =
 			return callback(error) if error?
 			if 200 <= response.statusCode < 300
 				callback null, data?.project, data?.privilegeLevel
+			else if response.statusCode == 429
+				logger.log(project_id, user_id, "rate-limit hit when joining project")
+				callback(new CodedError("rate-limit hit when joining project", "TooManyRequests"))
 			else
 				err = new Error("non-success status code from web: #{response.statusCode}")
-				logger.error {err, project_id, user_id}, "error accessing web api" 
+				logger.error {err, project_id, user_id}, "error accessing web api"
 				callback err

+ 17 - 0
services/real-time/test/acceptance/coffee/JoinProjectTests.coffee

@@ -89,3 +89,20 @@ describe "joinProject", ->
 			RealTimeClient.getConnectedClient @client.socket.sessionid, (error, client) =>
 				expect(@project_id in client.rooms).to.equal false
 				done()
+
+	describe "when over rate limit", ->
+		before (done) ->
+			async.series [
+				(cb) =>
+					@client = RealTimeClient.connect()
+					@client.on "connectionAccepted", cb
+
+				(cb) =>
+					@client.emit "joinProject", project_id: 'rate-limited', (@error) =>
+						cb()
+			], done
+
+		it "should return a TooManyRequests error code", ->
+			@error.message.should.equal "rate-limit hit when joining project"
+			@error.code.should.equal "TooManyRequests"
+

+ 9 - 6
services/real-time/test/acceptance/coffee/helpers/MockWebServer.coffee

@@ -19,12 +19,15 @@ module.exports = MockWebServer =
 	joinProjectRequest: (req, res, next) ->
 		{project_id} = req.params
 		{user_id} = req.query
-		MockWebServer.joinProject project_id, user_id, (error, project, privilegeLevel) ->
-			return next(error) if error?
-			res.json {
-				project: project
-				privilegeLevel: privilegeLevel
-			}
+		if project_id == 'rate-limited'
+			res.status(429).send()
+		else
+			MockWebServer.joinProject project_id, user_id, (error, project, privilegeLevel) ->
+				return next(error) if error?
+				res.json {
+					project: project
+					privilegeLevel: privilegeLevel
+				}
 	
 	running: false
 	run: (callback = (error) ->) ->

+ 10 - 0
services/real-time/test/unit/coffee/WebApiManagerTests.coffee

@@ -3,6 +3,7 @@ should = chai.should()
 sinon = require("sinon")
 modulePath = "../../../app/js/WebApiManager.js"
 SandboxedModule = require('sandboxed-module')
+{ CodedError } = require('../../../app/js/Errors')
 
 describe 'WebApiManager', ->
 	beforeEach ->
@@ -61,3 +62,12 @@ describe 'WebApiManager', ->
 					.calledWith(new Error("non-success code from web: 500"))
 					.should.equal true
 	
+		describe "when the project is over its rate limit", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 429}, null)
+				@WebApiManager.joinProject @project_id, @user_id, @callback
+
+			it "should call the callback with a TooManyRequests error code", ->
+				@callback
+					.calledWith(new CodedError("rate-limit hit when joining project", "TooManyRequests"))
+					.should.equal true