Merge e17b7b6 into 15d59a6

CHANGELOG

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Tue Feb 20 14:14:09 UTC 2018 - mvidner@suse.com
+
+- Distinguish between parsing and serializing in error reports.
+- Mention the file being parsed, and the position inside, in error
+  reports (bsc#1077435)
+- 0.6.3
+
 -------------------------------------------------------------------
 Fri May 26 11:46:40 UTC 2017 - jreidinger@suse.com
 

cfa.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |s|
   s.name        = "cfa"
-  s.version     = "0.6.2"
+  s.version     = "0.6.3"
   s.platform    = Gem::Platform::RUBY
   s.authors     = ["Josef Reidinger"]
   s.email       = ["jreidinger@suse.cz"]

lib/cfa/augeas_parser.rb

@@ -310,8 +310,9 @@ def initialize(lens)
     end
 
     # @param raw_string [String] a string to be parsed
+    # @param file_name [String] a file name, for error reporting ONLY
     # @return [AugeasTree] the parsed data
-    def parse(raw_string)
+    def parse(raw_string, file_name = nil)
       require "cfa/augeas_parser/reader"
       @old_content = raw_string
 
@@ -320,15 +321,17 @@ def parse(raw_string)
       root = load_path = nil
       Augeas.open(root, load_path, Augeas::NO_MODL_AUTOLOAD) do |aug|
         aug.set("/input", raw_string)
-        report_error(aug) unless aug.text_store(@lens, "/input", "/store")
+        report_error(aug, "parsing", file_name) \
+          unless aug.text_store(@lens, "/input", "/store")
 
         return AugeasReader.read(aug, "/store")
       end
     end
 
     # @param data [AugeasTree] the data to be serialized
+    # @param file_name [String] a file name, for error reporting ONLY
     # @return [String] a string to be written
-    def serialize(data)
+    def serialize(data, file_name = nil)
       require "cfa/augeas_parser/writer"
       # open augeas without any autoloading and it should not touch disk and
       # load lenses as needed only
@@ -339,7 +342,7 @@ def serialize(data)
         AugeasWriter.new(aug).write("/store", data)
 
         res = aug.text_retrieve(@lens, "/input", "/store", "/output")
-        report_error(aug) unless res
+        report_error(aug, "serializing", file_name) unless res
 
         return aug.get("/output")
       end
@@ -354,16 +357,30 @@ def empty
   private
 
     # @param aug [::Augeas]
-    def report_error(aug)
+    # @param activity ["parsing", "serializing"] for better error messages
+    # @param file_name [String,nil] a file name
+    def report_error(aug, activity, file_name)
       error = aug.error
       # zero is no error, so problem in lense
-      if aug.error[:code].nonzero?
-        raise "Augeas error #{error[:message]}. Details: #{error[:details]}."
+      if error[:code].nonzero?
+        raise "Augeas error: #{error[:message]}. Details: #{error[:details]}."
       end
 
-      msg = aug.get("/augeas/text/store/error/message")
-      location = aug.get("/augeas/text/store/error/lens")
-      raise "Augeas parsing/serializing error: #{msg} at #{location}"
+      file_name ||= "(unknown file)"
+      raise format("Augeas #{activity} error: %<message>s" \
+                   " at #{file_name}:%<line>s:%<char>s, lens %<lens>s",
+        aug_get_error(aug))
+    end
+
+    def aug_get_error(aug)
+      {
+        message: aug.get("/augeas/text/store/error/message"),
+        line:    aug.get("/augeas/text/store/error/line"),
+        char:    aug.get("/augeas/text/store/error/char"), # column
+        # file, line+column range, like
+        # "/usr/share/augeas/lenses/dist/hosts.aug:23.12-.42:"
+        lens:    aug.get("/augeas/text/store/error/lens")
+      }
     end
   end
 end

lib/cfa/base_model.rb

@@ -39,7 +39,7 @@ def initialize(parser, file_path, file_handler: nil)
     #   insertion of such values in the first place.
     def save(changes_only: false)
       merge_changes if changes_only
-      @file_handler.write(@file_path, @parser.serialize(data))
+      @file_handler.write(@file_path, @parser.serialize(data, @file_path))
     end
 
     # Reads a String using *file_handler*
@@ -51,7 +51,7 @@ def save(changes_only: false)
     # @raise a *parser* specific error. If the parsed String is malformed, then
     #   depending on the used parser it may raise an error.
     def load
-      self.data = @parser.parse(@file_handler.read(@file_path))
+      self.data = @parser.parse(@file_handler.read(@file_path), @file_path)
       @loaded = true
     end
 
@@ -88,6 +88,7 @@ def self.default_file_handler
     def self.default_file_handler=(value)
       @default_file_handler = value
     end
+    # rubocop:enable Style/TrivialAccessors
 
     # Generates accessors for trivial key-value attributes
     # @param attrs [Hash{Symbol => String}] mapping of methods to file keys

spec/augeas_parser_spec.rb

@@ -19,10 +19,18 @@
     end
 
     it "raises exception if augeas failed during parsing" do
-      example_file = "invalid syntax\n"
+      example_file = "root ALL=(ALL) ALL\ninvalid syntax\n"
 
-      msg = /Augeas parsing\/serializing error/
-      expect { subject.parse(example_file) }.to raise_error(msg)
+      msg = /Augeas parsing error: .* at \/dev\/garbage:2:0/
+      expect { subject.parse(example_file, "/dev/garbage") }.to raise_error(msg)
+    end
+
+    it "raises exception if augeas lens failed" do
+      example_file = "root ALL=(ALL) ALL\n"
+      bad_parser = described_class.new("nosuchlens.lns")
+
+      msg = /Augeas error: .* Details:/
+      expect { bad_parser.parse(example_file) }.to raise_error(msg)
     end
   end
 
@@ -44,8 +52,9 @@
       example_tree = CFA::AugeasTree.new
       example_tree["invalid"] = "test"
 
-      msg = /Augeas parsing\/serializing error/
-      expect { subject.serialize(example_tree) }.to raise_error(msg)
+      msg = /Augeas serializing error: .* at \/etc\/sudoers::/
+      expect { subject.serialize(example_tree, "/etc/sudoers") }
+        .to raise_error(msg)
     end
   end