Merge branch 'bug/2.7.x/better-error-reporting-in-face-cli-facade' into 2.7.x

3 files changed, 48 insertions, 55 deletions

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 69c3ad5ad..31e58aca4 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -212,63 +212,56 @@ class Puppet::Application::FaceBase < Puppet::Application
     status = false
 
     # Call the method associated with the provided action (e.g., 'find').
-    if @action
-      begin
-        # We need to do arity checking here because this is generic code
-        # calling generic methods – that have argument defaulting.  We need to
-        # make sure we don't accidentally pass the options as the first
-        # argument to a method that takes one argument.  eg:
-        #
-        #   puppet facts find
-        #   => options => {}
-        #      @arguments => [{}]
-        #   => @face.send :bar, {}
-        #
-        #   def face.bar(argument, options = {})
-        #   => bar({}, {})  # oops!  we thought the options were the
-        #                   # positional argument!!
-        #
-        # We could also fix this by making it mandatory to pass the options on
-        # every call, but that would make the Ruby API much more annoying to
-        # work with; having the defaulting is a much nicer convention to have.
-        #
-        # We could also pass the arguments implicitly, by having a magic
-        # 'options' method that was visible in the scope of the action, which
-        # returned the right stuff.
-        #
-        # That sounds attractive, but adds complications to all sorts of
-        # things, especially when you think about how to pass options when you
-        # are writing Ruby code that calls multiple faces.  Especially if
-        # faces are involved in that. ;)
-        #
-        # --daniel 2011-04-27
-        if (arity = @action.positional_arg_count) > 0
-          unless (count = arguments.length) == arity then
-            raise ArgumentError, "wrong number of arguments (#{count} for #{arity})"
-          end
-        end
-
-        result = @face.send(@action.name, *arguments)
-        puts render(result) unless result.nil?
-        status = true
-      rescue Exception => detail
-        puts detail.backtrace if Puppet[:trace]
-
-        case detail
-        when ArgumentError then
-          got, want = /\((\d+) for (\d+)\)/.match(detail.to_s).to_a.map {|x| x.to_i }
-          Puppet.err "puppet #{@face.name} #{@action.name}: #{want} argument expected but #{got} given"
-          Puppet.err "Try 'puppet help #{@face.name} #{@action.name}' for usage"
+    unless @action
+      puts Puppet::Face[:help, :current].help(@face.name)
+      raise "#{face} does not respond to action #{arguments.first}"
+    end
 
-        else # generic exception handling, alas.
-          Puppet.err detail.to_s
-        end
+    # We need to do arity checking here because this is generic code
+    # calling generic methods – that have argument defaulting.  We need to
+    # make sure we don't accidentally pass the options as the first
+    # argument to a method that takes one argument.  eg:
+    #
+    #   puppet facts find
+    #   => options => {}
+    #      @arguments => [{}]
+    #   => @face.send :bar, {}
+    #
+    #   def face.bar(argument, options = {})
+    #   => bar({}, {})  # oops!  we thought the options were the
+    #                   # positional argument!!
+    #
+    # We could also fix this by making it mandatory to pass the options on
+    # every call, but that would make the Ruby API much more annoying to
+    # work with; having the defaulting is a much nicer convention to have.
+    #
+    # We could also pass the arguments implicitly, by having a magic
+    # 'options' method that was visible in the scope of the action, which
+    # returned the right stuff.
+    #
+    # That sounds attractive, but adds complications to all sorts of
+    # things, especially when you think about how to pass options when you
+    # are writing Ruby code that calls multiple faces.  Especially if
+    # faces are involved in that. ;)
+    #
+    # --daniel 2011-04-27
+    if (arity = @action.positional_arg_count) > 0
+      unless (count = arguments.length) == arity then
+        s = arity == 2 ? '' : 's'
+        raise ArgumentError, "puppet #{@face.name} #{@action.name} takes #{arity-1} argument#{s}, but you gave #{count-1}"
       end
-    else
-      puts "#{face} does not respond to action #{arguments.first}"
-      puts Puppet::Face[:help, :current].help(@face.name)
     end
 
+    result = @face.send(@action.name, *arguments)
+    puts render(result) unless result.nil?
+    status = true
+
+  rescue Exception => detail
+    puts detail.backtrace if Puppet[:trace]
+    Puppet.err detail.to_s
+    Puppet.err "Try 'puppet help #{@face.name} #{@action.name}' for usage"
+
+  ensure
     exit status
   end
 end
diff --git a/lib/puppet/face/help/global.erb b/lib/puppet/face/help/global.erb
index f4c761b2b..a19ce1fb0 100644
--- a/lib/puppet/face/help/global.erb
+++ b/lib/puppet/face/help/global.erb
@@ -1,4 +1,4 @@
-puppet <subcommand> [options] <action> [options]
+Usage: puppet <subcommand> [options] <action> [options]
 
 Available subcommands, from Puppet Faces:
 % Puppet::Face.faces.sort.each do |name|
diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index 714d03f74..daf49e8de 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -65,7 +65,7 @@ module Puppet
           # return to the caller.  How strange we are. --daniel 2011-04-11
         else
           unless subcommand_name.nil? then
-            puts "Error: Unknown Puppet subcommand #{subcommand_name}.\n"
+            puts "Error: Unknown Puppet subcommand '#{subcommand_name}'"
           end
 
           # Doing this at the top of the file is natural, but causes puppet.rb