Make exec_hook robust to exceptions

The rationale for this is simple: you add hooks to enhance the core
functionality with added niceness, so you don't want a buggy hook to
break core functionality.

If you call exec_hook via an instance of Pry, it will additionally print
a warning to output whenever such an exception occurs, giving useful
information on how to debug the problem.

lib/pry/hooks.rb

@@ -20,6 +20,10 @@ class Pry
     end
     protected :hooks
 
+    def errors
+      @errors ||= []
+    end
+
     # Destructively merge the contents of two `Pry:Hooks` instances.
     # @param [Pry::Hooks] other The `Pry::Hooks` instance to merge
 
@@ -89,7 +93,14 @@ class Pry
       # silence warnings to get rid of 1.8's "warning: multiple values
       # for a block parameter" warnings
       Pry::Helpers::BaseHelpers.silence_warnings do
-        @hooks[event_name].map { |hook_name, callable| callable.call(*args, &block) }.last
+        @hooks[event_name].map do |hook_name, callable|
+          begin
+            callable.call(*args, &block)
+          rescue RescuableException => e
+            errors << e
+            e
+          end
+        end.last
       end
     end
 

lib/pry/pry_class.rb

@@ -116,7 +116,7 @@ class Pry
     pry_instance.backtrace = caller.tap(&:shift)
 
     # yield the binding_stack to the hook for modification
-    Pry.config.hooks.exec_hook(
+    pry_instance.exec_hook(
       :when_started,
       binding_stack = [target],
       options,

lib/pry/pry_instance.rb

@@ -142,7 +142,7 @@ class Pry
   # Initialize the repl session.
   # @param [Binding] target The target binding for the session.
   def repl_prologue(target)
-    hooks.exec_hook :before_session, output, target, self
+    exec_hook :before_session, output, target, self
     initialize_special_locals(target)
 
     @input_array << nil # add empty input so _in_ and _out_ match
@@ -154,7 +154,7 @@ class Pry
   # Clean-up after the repl session.
   # @param [Binding] target The target binding for the session.
   def repl_epilogue(target)
-    hooks.exec_hook :after_session, output, target, self
+    exec_hook :after_session, output, target, self
 
     Pry.active_sessions -= 1
     binding_stack.pop
@@ -234,7 +234,7 @@ class Pry
     result = set_last_exception(e, target)
   ensure
     update_input_history(code)
-    hooks.exec_hook :after_eval, result, self
+    exec_hook :after_eval, result, self
   end
 
   # Perform a read.
@@ -270,7 +270,7 @@ class Pry
 
     @suppress_output = true if eval_string =~ /;\Z/ || eval_string.empty?
 
-    hooks.exec_hook :after_read, eval_string, self
+    exec_hook :after_read, eval_string, self
     eval_string
   end
 
@@ -402,6 +402,25 @@ class Pry
     Pry::Command::VOID_VALUE
   end
 
+  # Execute the specified hook.
+  # @param [Symbol] name The hook name to execute
+  # @param [*Object] args The arguments to pass to the hook
+  # @return [Object, Exception] The return value of the hook or the exception raised
+  #
+  # If executing a hook raises an exception, we log that and then continue sucessfully.
+  # To debug such errors, use the global variable $pry_hook_error, which is set as a
+  # result.
+  def exec_hook(name, *args, &block)
+    e_before = hooks.errors.size
+    hooks.exec_hook(name, *args, &block)
+
+    hooks.errors[e_before..-1].each do |e|
+      output.puts "#{name} hook failed: #{e.class}: #{e.message}"
+      output.puts "#{e.backtrace.first}"
+      output.puts "(see _pry_.hooks.errors to debug)"
+    end
+  end
+
   # Set the last result of an eval.
   # This method should not need to be invoked directly.
   # @param [Object] result The result.

test/test_hooks.rb

@@ -290,6 +290,21 @@ describe Pry::Hooks do
       @hooks.add_hook(:test_hook, :my_name3) { 3 }
       @hooks.exec_hook(:test_hook).should == 3
     end
+
+    it 'should add exceptions to the errors array' do
+      @hooks.add_hook(:test_hook, :foo1) { raise 'one' }
+      @hooks.add_hook(:test_hook, :foo2) { raise 'two' }
+      @hooks.add_hook(:test_hook, :foo3) { raise 'three' }
+      @hooks.exec_hook(:test_hook)
+      @hooks.errors.map(&:message).should == ['one', 'two', 'three']
+    end
+
+    it 'should return the last exception raised as the return value' do
+      @hooks.add_hook(:test_hook, :foo1) { raise 'one' }
+      @hooks.add_hook(:test_hook, :foo2) { raise 'two' }
+      @hooks.add_hook(:test_hook, :foo3) { raise 'three' }
+      @hooks.exec_hook(:test_hook).should == @hooks.errors.last
+    end
   end
 
   describe "integration tests" do
@@ -335,6 +350,27 @@ describe Pry::Hooks do
         # cleanup after test
         Pry.config.exception_whitelist = old_ew
       end
+
+      describe "exceptions" do
+        before do
+          Pry.config.hooks.add_hook(:after_eval, :baddums){ raise "Baddums" }
+          Pry.config.hooks.add_hook(:after_eval, :simbads){ raise "Simbads" }
+        end
+
+        after do
+          Pry.config.hooks.delete_hook(:after_eval, :baddums)
+          Pry.config.hooks.delete_hook(:after_eval, :simbads)
+        end
+        it "should not raise exceptions" do
+          lambda{
+            mock_pry("1", "2", "3")
+          }.should.not.raise
+        end
+
+        it "should print out a notice for each exception raised" do
+          mock_pry("1").should =~ /after_eval hook failed: RuntimeError: Baddums\n.*after_eval hook failed: RuntimeError: Simbads/m
+        end
+      end
     end
 
   end