From 0d5ef629949e33b3a254be6f69516f87534d814a Mon Sep 17 00:00:00 2001
From: Behrad <behradz@gmail.com>
Date: Thu, 14 Jan 2016 18:51:10 +0330
Subject: [PATCH] guard attempts against passing max_attempts, should fix #797

---
 lib/queue/job.js | 31 ++++++++++++++++---------------
 test/test.js     | 24 ++++++++++++------------
 2 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/lib/queue/job.js b/lib/queue/job.js
index a6446962..cfcdcd53 100644
--- a/lib/queue/job.js
+++ b/lib/queue/job.js
@@ -188,8 +188,8 @@ exports.get = function( id, jobType, fn ) {
     job._delay            = hash.delay;
     job.priority(Number(hash.priority));
     job._progress         = hash.progress;
-    job._attempts         = hash.attempts;
-    job._max_attempts     = hash.max_attempts;
+    job._attempts         = Number(hash.attempts);
+    job._max_attempts     = Number(hash.max_attempts);
     job._state            = hash.state;
     job._error            = hash.error;
     job.created_at        = hash.created_at;
@@ -523,20 +523,19 @@ Job.prototype.priority = function( level ) {
  */
 
 Job.prototype.attempt = function( fn ) {
-  var self   = this
-    , client = this.client
+  var client = this.client
     , id     = this.id
     , key    = client.getKey('job:' + id);
 
-  //TODO this now can be removed, since max_attempts is set in the constructor
-  client.hsetnx(key, 'max_attempts', 1, function() {
-    client.hget(key, 'max_attempts', function( err, max ) {
-      client.hincrby(key, 'attempts', 1, function( err, attempts ) {
-        fn(err, Math.max(0, max - attempts), attempts, max);
-      });
-    });
-  });
-
+  this._attempts = this._attempts || 0;
+  if( this._attempts < this._max_attempts ) {
+    client.hincrby(key, 'attempts', 1, function( err, attempts ) {
+      this._attempts = attempts;
+      fn(err, Math.max(0, this._max_attempts - attempts), attempts, this._max_attempts);
+    }.bind(this));
+  } else {
+    fn(null, 0, this._attempts, this._max_attempts);
+  }
   return this;
 };
 
@@ -592,7 +591,7 @@ Job.prototype.failedAttempt = function( theErr, fn ) {
         this.emit( 'error', error );
         return fn && fn( error );
       }
-      if( remaining ) {
+      if( remaining > 0 ) {
         this.reattempt(attempts, function( err ) {
           if( err ) {
             this.emit( 'error', err );
@@ -600,8 +599,10 @@ Job.prototype.failedAttempt = function( theErr, fn ) {
           }
           fn && fn( err, true, attempts );
         }.bind(this));
-      } else {
+      } else if( remaining === 0 )  {
         fn && fn( null, false, attempts );
+      } else {
+        fn && fn( new Error('Attempts Exceeded') );
       }
     }.bind(this));
   }.bind(this));
diff --git a/test/test.js b/test/test.js
index d1700749..d1fab1b3 100755
--- a/test/test.js
+++ b/test/test.js
@@ -14,8 +14,8 @@ describe('CONNECTION', function(){
 		  redis: 'redis://localhost:6379/15?foo=bar'
 	  } );
 
-	  jobs.client.connection_options.port.should.be.eql( 6379 );
-	  jobs.client.connection_options.host.should.be.eql( 'localhost' );
+	  jobs.client.options.port.should.be.eql( 6379 );
+	  jobs.client.options.host.should.be.eql( 'localhost' );
 	  jobs.client.options.foo.should.be.eql( 'bar' );
 
 	  var jobData = {
@@ -46,8 +46,8 @@ describe('CONNECTION', function(){
 			}
 		} );
 
-		jobs.client.connection_options.port.should.be.eql( 6379 );
-		jobs.client.connection_options.host.should.be.eql( 'localhost' );
+		jobs.client.options.port.should.be.eql( 6379 );
+		jobs.client.options.host.should.be.eql( 'localhost' );
 		jobs.client.options.foo.should.be.eql( 'bar' );
 
 		var jobData = {
@@ -71,8 +71,8 @@ describe('CONNECTION', function(){
 			redis: 'redis://localhost:6379/?foo=bar'
 		} );
 
-		jobs.client.connection_options.port.should.be.eql( 6379 );
-		jobs.client.connection_options.host.should.be.eql( 'localhost' );
+		jobs.client.options.port.should.be.eql( 6379 );
+		jobs.client.options.host.should.be.eql( 'localhost' );
 		jobs.client.options.foo.should.be.eql( 'bar' );
 
 		var jobData = {
@@ -96,8 +96,8 @@ describe('CONNECTION', function(){
 			redis: 'redis://localhost:6379?foo=bar'
 		} );
 
-		jobs.client.connection_options.port.should.be.eql( 6379 );
-		jobs.client.connection_options.host.should.be.eql( 'localhost' );
+		jobs.client.options.port.should.be.eql( 6379 );
+		jobs.client.options.host.should.be.eql( 'localhost' );
 		jobs.client.options.foo.should.be.eql( 'bar' );
 
 		var jobData = {
@@ -127,8 +127,8 @@ describe('CONNECTION', function(){
 			}
 		} );
 
-		jobs.client.connection_options.port.should.be.eql( 6379 );
-		jobs.client.connection_options.host.should.be.eql( 'localhost' );
+		jobs.client.options.port.should.be.eql( 6379 );
+		jobs.client.options.host.should.be.eql( 'localhost' );
 		jobs.client.options.foo.should.be.eql( 'bar' );
 
 		var jobData = {
@@ -206,8 +206,8 @@ describe( 'JOBS', function () {
       redis: 'redis://localhost:6379/?foo=bar'
     } );
 
-    jobs.client.connection_options.port.should.be.eql( 6379 );
-    jobs.client.connection_options.host.should.be.eql( 'localhost' );
+    jobs.client.options.port.should.be.eql( 6379 );
+    jobs.client.options.host.should.be.eql( 'localhost' );
     jobs.client.options.foo.should.be.eql( 'bar' );
 
     var jobData = {