Merge pull request #26 from sharelatex/bg-error-check-on-unlock

improve error handling of releaseLock

services/track-changes/app/coffee/LockManager.coffee

@@ -3,6 +3,7 @@ redis = require("redis-sharelatex")
 rclient = redis.createClient(Settings.redis.web)
 os = require "os"
 crypto = require "crypto"
+logger = require "logger-sharelatex"
 
 HOST = os.hostname()
 PID = process.pid
@@ -57,7 +58,13 @@ module.exports = LockManager =
 				callback err, true
 
 	releaseLock: (key, lockValue, callback) ->
-		rclient.eval LockManager.unlockScript, 1, key, lockValue, callback
+		rclient.eval LockManager.unlockScript, 1, key, lockValue, (err, result) ->
+			if err?
+				return callback(err)
+			if result? and result isnt 1 # successful unlock should release exactly one key
+				logger.error {key:key, lockValue:lockValue, redis_err:err, redis_result:result}, "unlocking error"
+				return callback(new Error("tried to release timed out lock"))
+			callback(err,result)
 
 	runWithLock: (key, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) ->
 		LockManager.getLock key, (error, lockValue) ->

services/track-changes/test/unit/coffee/LockManager/LockManagerTests.coffee

@@ -15,6 +15,7 @@ describe "LockManager", ->
 				createClient: () => @rclient =
 					auth: sinon.stub()
 			"settings-sharelatex": @Settings
+			"logger-sharelatex": {error: ->}
 
 		@key = "lock-key"
 		@callback = sinon.stub()
@@ -174,3 +175,24 @@ describe "LockManager", ->
 
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
+
+		describe "releaseLock", ->
+			describe "when the lock is current", ->
+				beforeEach ->
+					@rclient.eval = sinon.stub().yields(null, 1)
+					@LockManager.releaseLock @key, @lockValue, @callback
+
+				it 'should clear the data from redis', ->
+					@rclient.eval.calledWith(@LockManager.unlockScript, 1, @key, @lockValue).should.equal true
+
+				it 'should call the callback', ->
+					@callback.called.should.equal true
+
+			describe "when the lock has expired", ->
+				beforeEach ->
+					@rclient.eval = sinon.stub().yields(null, 0)
+					@LockManager.releaseLock @key, @lockValue, @callback
+
+				it 'should return an error if the lock has expired', ->
+					@callback.calledWith(new Error("tried to release timed out lock")).should.equal true
+