[Pkg-puppet-devel] [facter] 291/352: (FACT-349) Surface error messages when rescuing exceptions

Stig Sandbeck Mathisen ssm at debian.org
Sun Apr 6 22:21:54 UTC 2014


This is an automated email from the git hooks/post-receive script.

ssm pushed a commit to branch master
in repository facter.

commit e07c48f9f8aff226e3e36c6efad72518af066659
Author: Adrien Thebo <git at somethingsinistral.net>
Date:   Thu Feb 13 10:21:26 2014 -0800

    (FACT-349) Surface error messages when rescuing exceptions
---
 lib/facter/application.rb     | 16 ++++++----------
 lib/facter/core/logging.rb    | 33 +++++++++++++++++++++++++++++++++
 lib/facter/core/resolvable.rb | 10 +++++-----
 lib/facter/util/collection.rb |  2 +-
 lib/facter/util/fact.rb       |  2 +-
 lib/facter/util/loader.rb     |  2 +-
 lib/facter/util/parser.rb     |  4 +---
 spec/unit/application_spec.rb | 15 ++++++++++++++-
 8 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/lib/facter/application.rb b/lib/facter/application.rb
index 9d51237..0618c20 100644
--- a/lib/facter/application.rb
+++ b/lib/facter/application.rb
@@ -11,7 +11,7 @@ module Facter
       begin
         Facter::Util::Config.ext_fact_loader = Facter::Util::DirectoryLoader.loader_for(dir)
       rescue Facter::Util::DirectoryLoader::NoSuchDirectoryError => error
-        $stderr.puts "Specified external facts directory #{dir} does not exist."
+        Facter.log_exception(error, "Specified external facts directory #{dir} does not exist.")
         exit(1)
       end
     end
@@ -36,8 +36,8 @@ module Facter
           begin
             facts[name] = Facter.value(name)
           rescue => error
-            $stderr.puts "Could not retrieve #{name}: #{error}"
-            exit 10
+            Facter.log_exception(error, "Could not retrieve #{name}: #{error.message}")
+            exit(10)
           end
         end
       end
@@ -61,12 +61,8 @@ module Facter
       exit(0)
 
     rescue => e
-      if options && options[:trace]
-        raise e
-      else
-        $stderr.puts "Error: #{e}"
-        exit(12)
-      end
+      Facter.log_exception(e)
+      exit(12)
     end
 
     private
@@ -151,7 +147,7 @@ OPTIONS
         opts.on("--plaintext",
                 "Emit facts in plaintext format.") { |v| options[:plaintext] = v }
         opts.on("--trace",
-                "Enable backtraces.")  { |v| options[:trace]  = v }
+                "Enable backtraces.")  { |v| Facter.trace(true) }
         opts.on("--external-dir DIR",
                 "The directory to use for external facts.") { |v| create_directory_loader(v) }
         opts.on("--no-external-dir",
diff --git a/lib/facter/core/logging.rb b/lib/facter/core/logging.rb
index 7c46d96..4cc2632 100644
--- a/lib/facter/core/logging.rb
+++ b/lib/facter/core/logging.rb
@@ -14,6 +14,9 @@ module Facter::Core::Logging
   # @api private
   @@timing = false
   # @api private
+  @@trace = false
+
+  # @api private
   @@warn_messages = {}
   # @api private
   @@debug_messages = {}
@@ -79,6 +82,28 @@ module Facter::Core::Logging
     end
   end
 
+  def log_exception(exception, message = :default)
+    self.warn(format_exception(exception, message, @@trace))
+  end
+
+  def format_exception(exception, message, trace)
+    arr = []
+
+    if message == :default
+      arr << exception.message
+    elsif message
+      arr << message
+    end
+
+    if trace
+      arr.concat(exception.backtrace)
+    end
+
+    arr.flatten.join("\n")
+  end
+
+  # Print an exception message, and optionally a backtrace if trace is set
+
   # Print timing information
   #
   # @param string [String] the time to print
@@ -125,6 +150,14 @@ module Facter::Core::Logging
     @@timing
   end
 
+  def trace(bool)
+    @@trace = bool
+  end
+
+  def trace?
+    @@trace
+  end
+
   # Clears the seen state of warning messages. See {warnonce}.
   #
   # @return [void]
diff --git a/lib/facter/core/resolvable.rb b/lib/facter/core/resolvable.rb
index 46bbad4..615aebe 100644
--- a/lib/facter/core/resolvable.rb
+++ b/lib/facter/core/resolvable.rb
@@ -66,13 +66,13 @@ module Facter::Core::Resolvable
 
     Facter::Util::Normalization.normalize(result)
   rescue Timeout::Error => detail
-    Facter.warn "Timed out after #{limit} seconds while resolving #{qualified_name}"
+    Facter.log_exception(detail, "Timed out after #{limit} seconds while resolving #{qualified_name}")
     return nil
-  rescue Facter::Util::Normalization::NormalizationError => e
-    Facter.warn "Fact resolution #{qualified_name} resolved to an invalid value: #{e.message}"
+  rescue Facter::Util::Normalization::NormalizationError => detail
+    Facter.log_exception(detail, "Fact resolution #{qualified_name} resolved to an invalid value: #{detail.message}")
     return nil
-  rescue => details
-    Facter.warn "Could not retrieve #{qualified_name}: #{details.message}"
+  rescue => detail
+    Facter.log_exception(detail, "Could not retrieve #{qualified_name}: #{detail.message}")
     return nil
   end
 
diff --git a/lib/facter/util/collection.rb b/lib/facter/util/collection.rb
index aac876f..be3f6f8 100644
--- a/lib/facter/util/collection.rb
+++ b/lib/facter/util/collection.rb
@@ -34,7 +34,7 @@ class Facter::Util::Collection
 
     fact
   rescue => e
-    Facter.warn "Unable to add fact #{name}: #{e}"
+    Facter.log_exception(e, "Unable to add fact #{name}: #{e}")
   end
 
   # Add a resolution mechanism for a named fact.  This does not distinguish
diff --git a/lib/facter/util/fact.rb b/lib/facter/util/fact.rb
index 0224ffc..304cbf7 100644
--- a/lib/facter/util/fact.rb
+++ b/lib/facter/util/fact.rb
@@ -69,7 +69,7 @@ class Facter::Util::Fact
 
     resolve
   rescue => e
-    Facter.warn "Unable to add resolve #{resolution_name.inspect} for fact #{@name}: #{e}"
+    Facter.log_exception(e, "Unable to add resolve #{resolution_name.inspect} for fact #{@name}: #{e.message}")
   end
 
   # Retrieve an existing resolution by name
diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb
index 4a71e1e..0f31bd5 100644
--- a/lib/facter/util/loader.rb
+++ b/lib/facter/util/loader.rb
@@ -117,7 +117,7 @@ class Facter::Util::Loader
       # Don't store the path if the file can't be loaded
       # in case it's loadable later on.
       @loaded.delete(file)
-      Facter.warn "Error loading fact #{file} #{detail}"
+      Facter.log_exception(detail, "Error loading fact #{file}: #{detail.message}")
     end
   end
 
diff --git a/lib/facter/util/parser.rb b/lib/facter/util/parser.rb
index 19f9d17..103be7c 100644
--- a/lib/facter/util/parser.rb
+++ b/lib/facter/util/parser.rb
@@ -55,9 +55,7 @@ module Facter::Util::Parser
     def results
       parse_results
     rescue Exception => detail
-      Facter.warn("Failed to handle #{filename} as #{self.class} facts")
-      Facter.warn("detail: #{detail.class}: #{detail.message}")
-      Facter.debug(detail.backtrace.join("\n\t"))
+      Facter.log_exception(detail, "Failed to handle #{filename} as #{self.class} facts: #{detail.message}")
       nil
     end
 
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index c881090..7f1e541 100644
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -8,7 +8,7 @@ describe Facter::Application do
       Facter::Application.parse(['architecture', 'kernel']).should == {}
     end
 
-    [:yaml, :json, :trace].each do |option_key|
+    [:yaml, :json].each do |option_key|
       it "sets options[:#{option_key}] when given --#{option_key}" do
         options = Facter::Application.parse(["--#{option_key}"])
         options[option_key].should be_true
@@ -31,6 +31,13 @@ describe Facter::Application do
       end
     end
 
+    it "enables tracing when given --trace" do
+      Facter.trace(false)
+      Facter::Application.parse(['--trace'])
+      Facter.should be_trace
+      Facter.trace(false)
+    end
+
     ['-t', '--timing'].each do |option|
       it "enables timing when given #{option}" do
         Facter.timing(false)
@@ -52,6 +59,12 @@ describe Facter::Application do
       Facter::Application.parse(argv)
       argv.should == ['uptime', 'virtual']
     end
+
+    after(:all) do
+      Facter.debugging(false)
+      Facter.timing(false)
+      Facter.trace(false)
+    end
   end
 
   describe "formatting facts" do

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-puppet/facter.git



More information about the Pkg-puppet-devel mailing list