immutable params

actionhero · Oct 16, 2015 · 3720771 · 3720771
1 parent 4d5f979
commit 3720771
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/lib/worker.js b/lib/worker.js
@@ -169,7 +169,8 @@ worker.prototype.perform = function(job, callback) {
             }else{
               args = [job.args];
             }
-            cb.apply(self, [].slice.call(args).concat([function(err, result){
+
+            var combinedInputs = [].slice.call(args).concat([function(err, result){
               returnCounter++;
               if(returnCounter !== 2){
                 self.emit('failure', self.queue, job, callbackError);
@@ -185,7 +186,14 @@ worker.prototype.perform = function(job, callback) {
                   }
                 });
               }
-            }]));
+            }]);
+
+            // When returning the payload back to redis (on error), it is important to make it immutable
+            // https://github.com/taskrabbit/node-resque/issues/99
+            // Note: if an input is a string or a number, you CANNOT freeze it saddly.
+            for(var i in combinedInputs){ Object.freeze(combinedInputs[i]); }
+
+            cb.apply(self, combinedInputs);
           }
         });
       }else{

diff --git a/test/core/worker.js b/test/core/worker.js
@@ -19,6 +19,12 @@ describe('worker', function(){
         callback(new Error("Blue Smoke"));
       }
     },
+    "messWithData": {
+      perform: function(a, callback){
+        a.data = 'new thing';
+        callback(null, a);
+      }
+    },
     "doubleCaller":{
       perform: function(callback){
         callback(null, 'a');
@@ -160,6 +166,17 @@ describe('worker', function(){
         worker.start();
       });
 
+      it('job arguments are immutable', function(done){
+        var listener = worker.on('success', function(q, job, result){
+          result.a.should.equal('starting value');
+          worker.removeAllListeners('success');
+          done();
+        });
+
+        queue.enqueue(specHelper.queue, "messWithData", {a: 'starting value'});
+        worker.start();
+      });
+
       it('can accept jobs that are simple functions', function(done){
         var listener = worker.on('success', function(q, job, result){
           result.should.equal("ok");
@@ -175,7 +192,7 @@ describe('worker', function(){
       it('will not work jobs that are not defined', function(done){
         var listener = worker.on('failure', function(q, job, failure){
           q.should.equal(specHelper.queue);
-          String(failure).should.equal == "Error: No job defined for class 'somethingFake'";
+          String(failure).should.equal("Error: No job defined for class 'somethingFake'");
 
           worker.removeAllListeners('failure');
           done();