Merge remote-tracking branch 'raykrueger/better_failure_hooks'

Author: defunkt

docs/HOOKS.md

@@ -86,7 +86,7 @@ The available hooks are:
   `Resque::Failure` backend.
 
 * `on_failure`: Called with the exception and job args if any exception occurs
-  while performing the job (or hooks).
+  while performing the job (or hooks), this includes Resque::DirtyExit.
 
 Hooks are easily implemented with superclasses or modules. A superclass could
 look something like this.

lib/resque.rb

@@ -222,19 +222,24 @@ def watch_queue(queue)
   #
   # If no queue can be inferred this method will raise a `Resque::NoQueueError`
   #
+  # Returns true if the job was queued, nil if the job was rejected by a 
+  # before_enqueue hook.
+  #
   # This method is considered part of the `stable` API.
   def enqueue(klass, *args)
     # Perform before_enqueue hooks. Don't perform enqueue if any hook returns false
     before_hooks = Plugin.before_enqueue_hooks(klass).collect do |hook|
       klass.send(hook, *args)
     end
-    return if before_hooks.any? { |result| result == false }
+    return nil if before_hooks.any? { |result| result == false }
 
     Job.create(queue_from_class(klass), klass, *args)
 
     Plugin.after_enqueue_hooks(klass).each do |hook|
       klass.send(hook, *args)
     end
+
+    return true
   end
 
   # This method can be used to conveniently remove a job from a queue.

lib/resque/job.rb

@@ -106,11 +106,6 @@ def perform
       job_args = args || []
       job_was_performed = false
 
-      before_hooks  = Plugin.before_hooks(job)
-      around_hooks  = Plugin.around_hooks(job)
-      after_hooks   = Plugin.after_hooks(job)
-      failure_hooks = Plugin.failure_hooks(job)
-
       begin
         # Execute before_perform hook. Abort the job gracefully if
         # Resque::DontPerform is raised.
@@ -158,7 +153,7 @@ def perform
       # If an exception occurs during the job execution, look for an
       # on_failure hook then re-raise.
       rescue Object => e
-        failure_hooks.each { |hook| job.send(hook, e, *job_args) }
+        run_failure_hooks(e)
         raise e
       end
     end
@@ -176,6 +171,7 @@ def args
     # Given an exception object, hands off the needed parameters to
     # the Failure module.
     def fail(exception)
+      run_failure_hooks(exception)
       Failure.create \
         :payload   => payload,
         :exception => exception,
@@ -201,5 +197,27 @@ def ==(other)
         payload_class == other.payload_class &&
         args == other.args
     end
+
+    def before_hooks
+      @before_hooks ||= Plugin.before_hooks(payload_class)
+    end
+
+    def around_hooks
+      @around_hooks ||= Plugin.around_hooks(payload_class)
+    end
+
+    def after_hooks
+      @after_hooks ||= Plugin.after_hooks(payload_class)
+    end
+
+    def failure_hooks 
+      @failure_hooks ||= Plugin.failure_hooks(payload_class)
+    end
+    
+    def run_failure_hooks(exception)
+      job_args = args || []
+      failure_hooks.each { |hook| payload_class.send(hook, exception, *job_args) }
+    end
+
   end
 end

test/job_hooks_test.rb

@@ -277,15 +277,15 @@ def self.perform(history)
   test "the before enqueue hook should run" do
     history = []
     @worker = Resque::Worker.new(:jobs)
-    Resque.enqueue(BeforeEnqueueJob, history)
+    assert Resque.enqueue(BeforeEnqueueJob, history)
     @worker.work(0)
     assert_equal history, [:before_enqueue], "before_enqueue was not run"
   end
 
   test "a before enqueue hook that returns false should prevent the job from getting queued" do
     history = []
     @worker = Resque::Worker.new(:jobs)
-    Resque.enqueue(BeforeEnqueueJobAbort, history)
+    assert_nil Resque.enqueue(BeforeEnqueueJobAbort, history)
     assert_equal 0, Resque.size(:jobs)
   end
 end

test/worker_test.rb

@@ -33,12 +33,30 @@
   end
 
   test "fails uncompleted jobs on exit" do
-    job = Resque::Job.new(:jobs, [GoodJob, "blah"])
+    job = Resque::Job.new(:jobs, {'class' => 'GoodJob', 'args' => "blah"})
     @worker.working_on(job)
     @worker.unregister_worker
     assert_equal 1, Resque::Failure.count
   end
 
+  class ::SimpleJobWithFailureHandling
+    def self.on_failure_record_failure(exception)
+      @@exception = exception
+    end
+    
+    def self.exception
+      @@exception
+    end
+  end
+
+  test "fails uncompleted jobs on exit, and calls failure hook" do
+    job = Resque::Job.new(:jobs, {'class' => 'SimpleJobWithFailureHandling', 'args' => ""})
+    @worker.working_on(job)
+    @worker.unregister_worker
+    assert_equal 1, Resque::Failure.count
+    assert(SimpleJobWithFailureHandling.exception.kind_of?(Resque::DirtyExit))
+  end
+
   test "can peek at failed jobs" do
     10.times { Resque::Job.create(:jobs, BadJob) }
     @worker.work(0)