Просмотр исходного кода

Return semantic error if the requested ops are not available when getting a document

James Allen 10 лет назад
Родитель
Сommit
11667e8250

+ 3 - 1
services/document-updater/app.coffee

@@ -74,10 +74,12 @@ app.get "/health_check/redis", (req, res, next)->
 
 
 app.use (error, req, res, next) ->
-	logger.error err: error, "request errored"
 	if error instanceof Errors.NotFoundError
 		res.send 404
+	else if error instanceof Errors.OpRangeNotAvailableError
+		res.send 422 # Unprocessable Entity
 	else
+		logger.error err: error, "request errored"
 		res.send(500, "Oops, something went wrong")
 
 shutdownCleanly = (signal) ->

+ 8 - 0
services/document-updater/app/coffee/Errors.coffee

@@ -5,6 +5,14 @@ NotFoundError = (message) ->
 	return error
 NotFoundError.prototype.__proto__ = Error.prototype
 
+OpRangeNotAvailableError = (message) ->
+	error = new Error(message)
+	error.name = "OpRangeNotAvailableError"
+	error.__proto__ = OpRangeNotAvailableError.prototype
+	return error
+OpRangeNotAvailableError.prototype.__proto__ = Error.prototype
+
 module.exports = Errors =
 	NotFoundError: NotFoundError
+	OpRangeNotAvailableError: OpRangeNotAvailableError
 

+ 4 - 3
services/document-updater/app/coffee/RedisManager.coffee

@@ -7,6 +7,7 @@ keys = require('./RedisKeyBuilder')
 logger = require('logger-sharelatex')
 metrics = require('./Metrics')
 ZipManager = require('./ZipManager')
+Errors = require "./Errors"
 
 redisOptions = _.clone(Settings.redis.web)
 redisOptions.return_buffers = true
@@ -125,8 +126,8 @@ module.exports = RedisManager =
 				first_version_in_redis = version - length
 
 				if start < first_version_in_redis or end > version
-					error = new Error("doc ops range is not loaded in redis")
-					logger.error err: error, length: length, version: version, start: start, end: end, "inconsistent version or length"
+					error = new Errors.OpRangeNotAvailableError("doc ops range is not loaded in redis")
+					logger.warn {err: error, doc_id, length, version, start, end}, "doc ops range is not loaded in redis"
 					return callback(error)
 
 				start = start - first_version_in_redis
@@ -135,7 +136,7 @@ module.exports = RedisManager =
 
 				if isNaN(start) or isNaN(end)
 					error = new Error("inconsistent version or lengths")
-					logger.error err: error, length: length, version: version, start: start, end: end, "inconsistent version or length"
+					logger.error {err: error, doc_id, length, version, start, end}, "inconsistent version or length"
 					return callback(error)
 
 				rclient.lrange keys.docOps(doc_id: doc_id), start, end, (error, jsonOps) ->

+ 19 - 8
services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee

@@ -70,7 +70,7 @@ describe "Getting a document", ->
 				lines: @lines = ["one", "two", "three"]
 			}
 
-			@updates = for v in [0..99]
+			@updates = for v in [0..199]
 				doc_id: @doc_id,
 				op: [i: v.toString(), p: 0]
 				v: v
@@ -78,16 +78,27 @@ describe "Getting a document", ->
 			DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) =>
 				throw error if error?
 				sinon.spy MockWebApi, "getDocument"
-				DocUpdaterClient.getDocAndRecentOps @project_id, @doc_id, 90, (error, res, @returnedDoc) => done()
+				done()
 
 		after ->
 			MockWebApi.getDocument.restore()
-
-		it "should return the recent ops", ->
-			@returnedDoc.ops.length.should.equal 10
-			for update, i in @updates.slice(90, -1)
-				@returnedDoc.ops[i].op.should.deep.equal update.op
-
+			
+		describe "when the ops are loaded", ->
+			before (done) ->
+				DocUpdaterClient.getDocAndRecentOps @project_id, @doc_id, 190, (error, res, @returnedDoc) => done()
+
+			it "should return the recent ops", ->
+				@returnedDoc.ops.length.should.equal 10
+				for update, i in @updates.slice(190, -1)
+					@returnedDoc.ops[i].op.should.deep.equal update.op
+					
+		describe "when the ops are not all loaded", ->
+			before (done) ->
+				# We only track 100 ops
+				DocUpdaterClient.getDocAndRecentOps @project_id, @doc_id, 10, (error, @res, @returnedDoc) => done()
+
+			it "should return UnprocessableEntity", ->
+				@res.statusCode.should.equal 422
 
 	describe "when the document does not exist", ->
 		before (done) ->