소스 검색

Do not log error on doc updater 404

Document updater 404s are not indicative of a problem, but just of the client
trying to connect to a deleted document, which is easy to trigger. We
log a warning instead.
Eric Mc Sween 7 년 전
부모
커밋
acf850bce9

+ 2 - 2
services/real-time/app/coffee/DocumentUpdaterManager.coffee

@@ -23,7 +23,7 @@ module.exports = DocumentUpdaterManager =
 				catch error
 				catch error
 					return callback(error)
 					return callback(error)
 				callback null, body?.lines, body?.version, body?.ranges, body?.ops
 				callback null, body?.lines, body?.version, body?.ranges, body?.ops
-			else if res.statusCode == 422 # Unprocessable Entity
+			else if res.statusCode in [404, 422]
 				err = new Error("doc updater could not load requested ops")
 				err = new Error("doc updater could not load requested ops")
 				err.statusCode = res.statusCode
 				err.statusCode = res.statusCode
 				logger.warn {err, project_id, doc_id, url, fromVersion}, "doc updater could not load requested ops"
 				logger.warn {err, project_id, doc_id, url, fromVersion}, "doc updater could not load requested ops"
@@ -51,7 +51,7 @@ module.exports = DocumentUpdaterManager =
 				err.statusCode = res.statusCode
 				err.statusCode = res.statusCode
 				logger.error {err, project_id}, "document updater returned failure status code: #{res.statusCode}"
 				logger.error {err, project_id}, "document updater returned failure status code: #{res.statusCode}"
 				return callback(err)
 				return callback(err)
-				
+
 	queueChange: (project_id, doc_id, change, callback = ()->)->
 	queueChange: (project_id, doc_id, change, callback = ()->)->
 		jsonChange = JSON.stringify change
 		jsonChange = JSON.stringify change
 		if jsonChange.indexOf("\u0000") != -1
 		if jsonChange.indexOf("\u0000") != -1

+ 23 - 22
services/real-time/test/unit/coffee/DocumentUpdaterManagerTests.coffee

@@ -10,13 +10,13 @@ describe 'DocumentUpdaterManager', ->
 		@doc_id = "doc-id-394"
 		@doc_id = "doc-id-394"
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
 		@version = 42
 		@version = 42
-		@settings = 
+		@settings =
 			apis: documentupdater: url: "http://doc-updater.example.com"
 			apis: documentupdater: url: "http://doc-updater.example.com"
 			redis: documentupdater:
 			redis: documentupdater:
 				key_schema:
 				key_schema:
 					pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}"
 					pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}"
 		@rclient = {auth:->}
 		@rclient = {auth:->}
-			
+
 		@DocumentUpdaterManager = SandboxedModule.require modulePath,
 		@DocumentUpdaterManager = SandboxedModule.require modulePath,
 			requires:
 			requires:
 				'settings-sharelatex':@settings
 				'settings-sharelatex':@settings
@@ -59,17 +59,19 @@ describe 'DocumentUpdaterManager', ->
 			it "should return an error to the callback", ->
 			it "should return an error to the callback", ->
 				@callback.calledWith(@error).should.equal true
 				@callback.calledWith(@error).should.equal true
 
 
-		describe "when the document updater returns a 422 status code", ->
-			beforeEach ->
-				@request.get = sinon.stub().callsArgWith(1, null, { statusCode: 422 }, "")
-				@DocumentUpdaterManager.getDocument @project_id, @doc_id, @fromVersion, @callback
+		[404, 422].forEach (statusCode) ->
+			describe "when the document updater returns a #{statusCode} status code", ->
+				beforeEach ->
+					@request.get = sinon.stub().callsArgWith(1, null, { statusCode }, "")
+					@DocumentUpdaterManager.getDocument @project_id, @doc_id, @fromVersion, @callback
 
 
-			it "should return the callback with an error", ->
-				err = new Error("doc updater could not load requested ops")
-				err.statusCode = 422
-				@callback
-					.calledWith(err)
-					.should.equal true
+				it "should return the callback with an error", ->
+					@callback.called.should.equal(true)
+					err = @callback.getCall(0).args[0]
+					err.should.have.property('statusCode', statusCode)
+					err.should.have.property('message', "doc updater could not load requested ops")
+					@logger.error.called.should.equal(false)
+					@logger.warn.called.should.equal(true)
 
 
 		describe "when the document updater returns a failure error code", ->
 		describe "when the document updater returns a failure error code", ->
 			beforeEach ->
 			beforeEach ->
@@ -77,11 +79,11 @@ describe 'DocumentUpdaterManager', ->
 				@DocumentUpdaterManager.getDocument @project_id, @doc_id, @fromVersion, @callback
 				@DocumentUpdaterManager.getDocument @project_id, @doc_id, @fromVersion, @callback
 
 
 			it "should return the callback with an error", ->
 			it "should return the callback with an error", ->
-				err = new Error("doc updater returned failure status code: 500")
-				err.statusCode = 500
-				@callback
-					.calledWith(err)
-					.should.equal true
+				@callback.called.should.equal(true)
+				err = @callback.getCall(0).args[0]
+				err.should.have.property('statusCode', 500)
+				err.should.have.property('message', "doc updater returned a non-success status code: 500")
+				@logger.error.called.should.equal(true)
 
 
 	describe 'flushProjectToMongoAndDelete', ->
 	describe 'flushProjectToMongoAndDelete', ->
 		beforeEach ->
 		beforeEach ->
@@ -113,11 +115,10 @@ describe 'DocumentUpdaterManager', ->
 				@DocumentUpdaterManager.flushProjectToMongoAndDelete @project_id, @callback
 				@DocumentUpdaterManager.flushProjectToMongoAndDelete @project_id, @callback
 
 
 			it "should return the callback with an error", ->
 			it "should return the callback with an error", ->
-				err = new Error("doc updater returned failure status code: 500")
-				err.statusCode = 500
-				@callback
-					.calledWith(err)
-					.should.equal true
+				@callback.called.should.equal(true)
+				err = @callback.getCall(0).args[0]
+				err.should.have.property('statusCode', 500)
+				err.should.have.property('message', "document updater returned a failure status code: 500")
 
 
 	describe 'queueChange', ->
 	describe 'queueChange', ->
 		beforeEach ->
 		beforeEach ->