fix(retry-job): throw error when job is not in active state (#2741)

Author: roggervalf

lib/commands/moveToFinished-9.lua

@@ -35,6 +35,7 @@
       0 OK
       -1 Missing key.
       -2 Missing lock.
+      -3 - Job not in active set.
 
      Events:
       'completed/failed'
@@ -81,15 +82,13 @@ local function collectMetrics(metaKey, dataPointsList, maxDataPoints, timestamp)
     end
 end
 
+-- Includes
+--- @include "includes/removeLock"
+
 if rcall("EXISTS", KEYS[3]) == 1 then -- // Make sure job exists
-    if ARGV[5] ~= "0" then
-        local lockKey = KEYS[3] .. ':lock'
-        if rcall("GET", lockKey) == ARGV[5] then
-            rcall("DEL", lockKey)
-            rcall("SREM", KEYS[8], ARGV[1])
-        else
-            return -2
-        end
+    local errorCode = removeLock(KEYS[3], KEYS[8], ARGV[5], ARGV[1])
+    if errorCode < 0 then
+        return errorCode
     end
 
     -- Remove from active list (if not active we shall return error)

lib/commands/retryJob-7.lua

@@ -21,27 +21,23 @@
      0  - OK
      -1 - Missing key
      -2 - Job Not locked
+     -3 - Job not in active set
 ]]
 local rcall = redis.call
 
 -- Includes
 --- @include "includes/addJobWithPriority"
 --- @include "includes/getTargetQueueList"
+--- @include "includes/removeLock"
 
 if rcall("EXISTS", KEYS[3]) == 1 then
-
-  -- Check for job lock
-  if ARGV[3] ~= "0" then
-    local lockKey = KEYS[3] .. ':lock'
-    if rcall("GET", lockKey) == ARGV[3] then
-      rcall("DEL", lockKey)
-      rcall("SREM", KEYS[6], ARGV[2])
-    else
-      return -2
-    end
+  local errorCode = removeLock(KEYS[3], KEYS[6], ARGV[3], ARGV[2])
+  if errorCode < 0 then
+    return errorCode
   end
 
-  rcall("LREM", KEYS[1], 0, ARGV[2])
+  local numRemovedElements = rcall("LREM", KEYS[1], -1, ARGV[2])
+  if numRemovedElements < 1 then return -3 end
 
   local target = getTargetQueueList(KEYS[4], KEYS[2], KEYS[5])
 

test/test_job.js

@@ -663,6 +663,9 @@ describe('Job', () => {
           .then(isFailed => {
             expect(isFailed).to.be(false);
           })
+          .then(() => {
+            return scripts.moveToActive(queue);
+          })
           .then(() => {
             return job.moveToFailed(new Error('test error'), true);
           })