Improve error output on extract_messages failure

The `extract_messages` helper was designed to fail loudly if the
input didn't match what was expected by the regex. The goal was to help
hook authors debug and catch these issues quickly and easily.

However, there are some situations where hook authors did nothing wrong,
for example if you are using `rbenv` and on a version of Ruby where a
gem hasn't yet been installed (but the shim already exists), you'll get
an error like the following:

    The `rubocop' command exists in these Ruby versions:
      1.9.3-p551
      2.1.1
      2.1.2
      2.2.1
      2.2.2
      2.3.0
      jruby-1.7.20

...which appears like the following in the hook error output:

    Analyzing with RuboCop..............................[RuboCop] FAILED
    Hook raised unexpected error
    Unexpected output: unable to determine line number or type of
    error/warning for message ''
    lib/overcommit/hook/pre_commit/base.rb:31:in `block in extract_messages'
    lib/overcommit/hook/pre_commit/base.rb:28:in `map'
    lib/overcommit/hook/pre_commit/base.rb:28:in `extract_messages'
    lib/overcommit/hook/pre_commit/rubo_cop.rb:19:in `run'
    lib/overcommit/hook/base.rb:45:in `block in run_and_transform'
    lib/overcommit/utils.rb:259:in `with_environment'
    lib/overcommit/hook/base.rb:45:in `run_and_transform'
    lib/overcommit/hook_runner.rb:152:in `run_hook'
    lib/overcommit/hook_runner.rb:88:in `block in consume'
    lib/overcommit/hook_runner.rb:85:in `loop'
    lib/overcommit/hook_runner.rb:85:in `consume'

The stacktrace in this case is unsightly and doesn't explain what's
actually going on.

Fix this by throwing a special class of error and printing the remaining
unprocessed output.

Fixes sds#335

Author: sds

CHANGELOG.md

@@ -6,6 +6,8 @@
   Unicode characters could cause a crash due to invalid byte sequences
 * Add `ForbiddenBranches` pre-commit hook which prevents creating a commit
   on any blacklisted branch by name/pattern
+* Improve error message output when there is a problem processing messages
+  via `extract_messages` pre-commit hook helper
 
 ## 0.32.0.rc1
 

lib/overcommit/exceptions.rb

@@ -42,6 +42,9 @@ class InvalidHookDefinition < StandardError; end
   # Raised when one or more hook plugin signatures have changed.
   class InvalidHookSignature < StandardError; end
 
+  # Raised when there is a problem processing output into {Hook::Messages}s.
+  class MessageProcessingError < StandardError; end
+
   # Raised when an installation target already contains non-Overcommit hooks.
   class PreExistingHooks < StandardError; end
 end

lib/overcommit/hook/pre_commit/base.rb

@@ -22,13 +22,16 @@ class Base < Overcommit::Hook::Base
     # @param type_categorizer [Proc] function executed against the `type`
     #   capture group to convert it to a `:warning` or `:error` symbol. Assumes
     #   `:error` if `nil`.
-    # @raise [RuntimeError] line of output did not match regex
+    # @raise [Overcommit::Exceptions::MessageProcessingError] line of output did
+    #   not match regex
     # @return [Array<Message>]
     def extract_messages(output_messages, regex, type_categorizer = nil)
-      output_messages.map do |message|
+      output_messages.map.with_index do |message, index|
         unless match = message.match(regex)
-          raise 'Unexpected output: unable to determine line number or type ' \
-                "of error/warning for message '#{message}'"
+          raise Overcommit::Exceptions::MessageProcessingError,
+                'Unexpected output: unable to determine line number or type ' \
+                "of error/warning for output:\n" \
+                "#{output_messages[index..-1].join("\n")}"
         end
 
         file = extract_file(match, message)
@@ -43,7 +46,8 @@ def extract_file(match, message)
       return unless match.names.include?('file')
 
       if match[:file].to_s.empty?
-        raise "Unexpected output: no file found in '#{message}'"
+        raise Overcommit::Exceptions::MessageProcessingError,
+              "Unexpected output: no file found in '#{message}'"
       end
 
       match[:file]
@@ -53,15 +57,17 @@ def extract_line(match, message)
       return unless match.names.include?('line')
       Integer(match[:line])
     rescue ArgumentError, TypeError
-      raise "Unexpected output: invalid line number found in '#{message}'"
+      raise Overcommit::Exceptions::MessageProcessingError,
+            "Unexpected output: invalid line number found in '#{message}'"
     end
 
     def extract_type(match, message, type_categorizer)
       if type_categorizer
         type_match = match.names.include?('type') ? match[:type] : nil
         type = type_categorizer.call(type_match)
         unless Overcommit::Hook::MESSAGE_TYPES.include?(type)
-          raise "Invalid message type '#{type}' for '#{message}': must " \
+          raise Overcommit::Exceptions::MessageProcessingError,
+                "Invalid message type '#{type}' for '#{message}': must " \
                 "be one of #{Overcommit::Hook::MESSAGE_TYPES.inspect}"
         end
         type

lib/overcommit/hook_runner.rb

@@ -150,6 +150,9 @@ def run_hook(hook) # rubocop:disable Metrics/CyclomaticComplexity
         return if should_skip?(hook)
 
         status, output = hook.run_and_transform
+      rescue Overcommit::Exceptions::MessageProcessingError => ex
+        status = :fail
+        output = ex.message
       rescue => ex
         status = :fail
         output = "Hook raised unexpected error\n#{ex.message}\n#{ex.backtrace.join("\n")}"