diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index b0813e47f9..41c7a01958 100644 --- a/services/document-updater/app.coffee +++ b/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) -> diff --git a/services/document-updater/app/coffee/Errors.coffee b/services/document-updater/app/coffee/Errors.coffee index 4a29822efc..941bfcc9f1 100644 --- a/services/document-updater/app/coffee/Errors.coffee +++ b/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 diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index d280de1cea..b7ebb2cd4f 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/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) -> diff --git a/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee index 35c3f2c55a..210502ae45 100644 --- a/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee +++ b/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() + + 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(90, -1) - @returnedDoc.ops[i].op.should.deep.equal update.op + 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) ->