Improve error messages across the board

* Replace puts calls with appropriate logger
* Add CustomErrors module for better end user feedback for pipeline
parsing errors
* Raise errors when and where they happen instead of waiting
* Add pry support for easier debugging

jenkins_pipeline_builder.gemspec

@@ -40,6 +40,7 @@ automating Job & Pipeline creation from the YAML files checked-in with your appl
   spec.add_development_dependency 'equivalent-xml'
   spec.add_development_dependency 'yard-thor'
   spec.add_development_dependency 'yard'
+  spec.add_development_dependency 'pry'
   spec.add_development_dependency 'rspec_junit_formatter'
   spec.add_development_dependency 'webmock', '~> 1.0'
   spec.add_development_dependency 'rubocop', '= 0.40.0'

lib/jenkins_pipeline_builder.rb

@@ -25,6 +25,7 @@
 
 require 'jenkins_pipeline_builder/version'
 require 'jenkins_pipeline_builder/utils'
+require 'jenkins_pipeline_builder/custom_errors'
 require 'jenkins_pipeline_builder/compiler'
 require 'jenkins_pipeline_builder/module_registry'
 require 'jenkins_pipeline_builder/pull_request_generator'

lib/jenkins_pipeline_builder/custom_errors.rb

@@ -0,0 +1,20 @@
+module CustomErrors
+  class ParseError < StandardError
+    def initialize(msg, path = nil)
+      super(format_msg(msg, path).squeeze(' '))
+    end
+
+    private
+
+    def format_msg(msg, path)
+      if path.nil?
+        %(There was an error while parsing a file:
+        #{msg})
+      else
+        %(There was an error while parsing a file:
+        #{path}
+        #{msg})
+      end
+    end
+  end
+end

lib/jenkins_pipeline_builder/extension_dsl.rb

@@ -6,7 +6,7 @@
 
     JenkinsPipelineBuilder.registry.register([:job, type], set)
     versions = set.extensions.map(&:min_version)
-    puts "Successfully registered #{set.name} for versions #{versions}" if set.announced
+    JenkinsPipelineBuilder.logger.info "Successfully registered #{set.name} for versions #{versions}" if set.announced
     true
   end
 end
@@ -17,6 +17,6 @@ def job_attribute(&block)
 
   JenkinsPipelineBuilder.registry.register([:job], set)
   versions = set.extensions.map(&:min_version)
-  puts "Successfully registered #{set.name} for versions #{versions}" if set.announced
+  JenkinsPipelineBuilder.logger.info "Successfully registered #{set.name} for versions #{versions}" if set.announced
   true
 end

lib/jenkins_pipeline_builder/extension_set.rb

@@ -66,7 +66,8 @@ def extension
       extension = versions[highest_allowed_version]
 
       unless extension
-        raise "Can't find version of #{name} lte #{installed_version}, available versions: #{versions.keys.map(&:to_s)}"
+        raise %(Can't find version of #{name} lte #{installed_version}.
+          Available versions: #{versions.keys.map(&:to_s)})
       end
       extension
     end
@@ -79,7 +80,7 @@ def merge(other_set)
         mismatch << "The values for #{method_name} do not match '#{val1}' : '#{val2}'" unless val1 == val2
       end
       mismatch.each do |error|
-        puts error
+        logger.error error
       end
       raise 'Values did not match, cannot merge extension sets' if mismatch.any?
 
@@ -129,8 +130,8 @@ def valid?
       valid = errors.empty?
       unless valid
         name ||= 'A plugin with no name provided'
-        puts "Encountered errors while registering #{name}"
-        puts errors.map { |k, v| "#{k}: #{v}" }.join(', ')
+        logger.error "Encountered errors while registering #{name}"
+        logger.error errors.map { |k, v| "#{k}: #{v}" }.join(', ')
       end
       valid
     end
@@ -154,6 +155,10 @@ def versions
 
     private
 
+    def logger
+      JenkinsPipelineBuilder.logger
+    end
+
     def highest_allowed_version
       ordered_version_list.each do |version|
         return version if version <= installed_version
@@ -173,7 +178,8 @@ def ordered_version_list
     end
 
     def deprecation_warning(name, block)
-      puts "WARNING: #{name} set the version in the #{block} block, this is deprecated. Please use a version block."
+      logger.warn %(WARNING: #{name} set the version in the #{block} block, this is deprecated.
+      Please use a version block.)
     end
   end
 end

lib/jenkins_pipeline_builder/extensions.rb

@@ -58,10 +58,11 @@ def valid?
 
     def execute(value, n_xml)
       errors = check_parameters value
-      errors.each do |error|
-        JenkinsPipelineBuilder.logger.error error
-      end
-      return false if errors.any?
+      raise ArgumentError, errors.join("\n") if errors.any?
+      raise ArgumentError, %(Extension #{name} has no valid path
+      Check ModuleRegistry#entries and the definition of the extension
+      Note: job_attributes have no implicit path and must be set in the builder
+      ).squeeze(' ') unless path
 
       n_builders = n_xml.xpath(path).first
       n_builders.instance_exec(value, &before) if before

lib/jenkins_pipeline_builder/generator.rb

@@ -101,6 +101,7 @@ def resolve_project(project)
       project[:settings] = compiler.get_settings_bag(project, settings)
 
       errors = process_project project
+
       print_project_errors errors
       return false, 'Encountered errors exiting' unless errors.empty?
 
@@ -143,8 +144,8 @@ def print_compile_errors(errors)
 
     def print_project_errors(errors)
       errors.each do |error|
-        puts 'Encountered errors processing:'
-        puts error.inspect
+        logger.error 'Encountered errors processing:'
+        logger.error error.inspect
       end
     end
 

lib/jenkins_pipeline_builder/job.rb

@@ -111,9 +111,7 @@ def setup_freestyle_base(params)
         raise "Job template '#{template_name}' can't be resolved." unless @job_templates.key?(template_name)
         params.delete(:template)
         template = @job_templates[template_name]
-        puts "Template found: #{template}"
         params = template.deep_merge(params)
-        puts "Template merged: #{template}"
       end
 
       xml = JenkinsPipelineBuilder.client.job.build_freestyle_config(params)

lib/jenkins_pipeline_builder/job_collection.rb

@@ -76,7 +76,7 @@ def load_file(path, remote = false)
         load_section section, remote
       end
     rescue StandardError => err
-      raise "There was an error while parsing a file #{err.message}"
+      raise CustomErrors::ParseError.new err.message, path
     end
 
     def load_section(section, remote)
@@ -88,6 +88,12 @@ def load_section(section, remote)
         remote_dependencies.load value
         return
       end
+
+      raise TypeError, %(Expected Hash received #{value.class}.
+        Verify that the pipeline section is made up of a single {key: Hash/Object} pair
+        See the definition for:
+        \t#{section}).squeeze(' ') unless value.is_a? Hash
+
       name = value[:name]
       process_collection! name, key, value, remote
     end

spec/lib/jenkins_pipeline_builder/generator_spec.rb

@@ -219,13 +219,13 @@ def fixture_path(fixture)
     it 'errors when reading a bad yaml file' do
       path = File.expand_path('../fixtures/generator_tests/test_bad_yaml_files', __FILE__)
       expect { @generator.job_collection.load_from_path path }.to raise_error(
-        RuntimeError, /There was an error while parsing a file/
+        CustomErrors::ParseError, /There was an error while parsing a file/
       )
     end
     it 'errors when reading a bad json file' do
       path = File.expand_path('../fixtures/generator_tests/test_bad_json_files', __FILE__)
       expect { @generator.job_collection.load_from_path path }.to raise_error(
-        RuntimeError, /There was an error while parsing a file/
+        CustomErrors::ParseError, /There was an error while parsing a file/
       )
     end
   end