Move CatalogError, BootstrapError to error class

Author: Kevin Paulisse

doc/dev/api/v1/catalog.md

@@ -2,6 +2,12 @@
 
 ## Overview
 
-`catalog` allows you to build a catalog using the octocatalog-diff compiler or to obtain a catalog from a Puppet server.
+`catalog` returns an `OctocatalogDiff::Catalog` object built with the octocatalog-diff compiler, obtained from a Puppet server, or read in from a file. This is analogous to using the `--catalog-only` option with the octocatalog-diff command-line script.
 
-This is analogous to using the `--catalog-only` option with the octocatalog-diff command-line script.
+```
+catalog_obj = OctocatalogDiff::API::V1.catalog(
+  filename: "String",
+  logger: Logger,
+  test: <true|false>
+)
+```

lib/octocatalog-diff/catalog-util/bootstrap.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative '../bootstrap'
+require_relative '../errors'
 require_relative '../util/parallel'
 require_relative 'git'
 
@@ -12,10 +13,6 @@ module CatalogUtil
     # parallelization of bootstrap, and formats arguments as expected by the higher level bootstrap
     # script.
     class Bootstrap
-      # Exceptions that are anticipated can be caught in the calling class and tested
-      # for explicitly in spec tests.
-      class BootstrapError < RuntimeError; end
-
       # Bootstrap directories specified by --bootstrapped-from-dir and --bootstrapped-to-dir
       # command line options. Bootstrapping occurs in parallel. This takes no parameters (options come
       # from options) and returns nothing (it raises an exception if something fails).
@@ -28,7 +25,7 @@ def self.bootstrap_directory_parallelizer(options, logger)
             message = 'Must specify a from-branch other than . when using --bootstrapped-from-dir!' \
                       ' Please use "-f <from_branch>" argument.'
             logger.error(message)
-            raise BootstrapError, message
+            raise OctocatalogDiff::Errors::BootstrapError, message
           end
 
           opts = options.merge(branch: options[:from_env],
@@ -43,7 +40,7 @@ def self.bootstrap_directory_parallelizer(options, logger)
             message = 'Must specify a to-branch other than . when using --bootstrapped-to-dir!' \
                       ' Please use "-t <to_branch>" argument.'
             logger.error(message)
-            raise BootstrapError, message
+            raise OctocatalogDiff::Errors::BootstrapError, message
           end
 
           opts = options.merge(branch: options[:to_env],
@@ -58,7 +55,7 @@ def self.bootstrap_directory_parallelizer(options, logger)
           message = 'Specify one or more of --bootstrapped-from-dir / --bootstrapped-to-dir / --cached-master-dir' \
                     ' when using --bootstrap_then_exit'
           logger.error(message)
-          raise BootstrapError, message
+          raise OctocatalogDiff::Errors::BootstrapError, message
         end
 
         # Bootstrap the directories in parallel. Since there are no results here that we
@@ -78,7 +75,7 @@ def self.bootstrap_directory_parallelizer(options, logger)
             logger.debug("Success bootstrap_directory for #{result.args[:tag]}")
           else
             errmsg = "Failed bootstrap_directory for #{result.args[:tag]}: #{result.exception.class} #{result.exception.message}"
-            raise BootstrapError, errmsg
+            raise OctocatalogDiff::Errors::BootstrapError, errmsg
           end
         end
       end
@@ -106,7 +103,7 @@ def self.git_checkout(logger, dir_opts)
         logger.debug("Success git checkout #{dir_opts[:basedir]}:#{dir_opts[:branch]} -> #{dir_opts[:path]}")
       rescue OctocatalogDiff::CatalogUtil::Git::GitCheckoutError => exc
         logger.error("Git checkout error: #{exc}")
-        raise BootstrapError, exc
+        raise OctocatalogDiff::Errors::BootstrapError, exc
       end
 
       # Install bootstrap script in the target directory. This allows proper bootstrapping from the
@@ -121,7 +118,7 @@ def self.install_bootstrap_script(logger, opts)
         else
           File.join(opts[:basedir], opts[:bootstrap_script])
         end
-        raise BootstrapError, "Bootstrap script #{src} does not exist" unless File.file?(src)
+        raise OctocatalogDiff::Errors::BootstrapError, "Bootstrap script #{src} does not exist" unless File.file?(src)
 
         logger.debug('Begin install bootstrap script in target directory')
 
@@ -146,7 +143,9 @@ def self.run_bootstrap(logger, opts)
           output = result[:output].split(/[\r\n]+/)
           output.each { |x| logger.debug("Bootstrap: #{x}") }
         end
-        raise BootstrapError, "bootstrap failed for #{opts[:path]}: #{result[:output]}" unless (result[:status_code]).zero?
+        unless (result[:status_code]).zero?
+          raise OctocatalogDiff::Errors::BootstrapError, "bootstrap failed for #{opts[:path]}: #{result[:output]}"
+        end
         logger.debug("Success bootstrap in #{opts[:path]}")
         result[:output]
       end

lib/octocatalog-diff/cli.rb

@@ -3,6 +3,7 @@
 require_relative 'api/v1/catalog-compile'
 require_relative 'api/v1/catalog-diff'
 require_relative 'catalog-util/cached_master_directory'
+require_relative 'errors'
 require_relative 'util/catalogs'
 require_relative 'version'
 
@@ -176,7 +177,7 @@ def self.catalog_only(logger, options)
     def self.bootstrap_then_exit(logger, catalogs_obj)
       catalogs_obj.bootstrap_then_exit
       return EXITCODE_SUCCESS_NO_DIFFS
-    rescue OctocatalogDiff::Util::Catalogs::BootstrapError => exc
+    rescue OctocatalogDiff::Errors::BootstrapError => exc
       logger.fatal("--bootstrap-then-exit error: bootstrap failed (#{exc})")
       return EXITCODE_FAILURE
     end

lib/octocatalog-diff/errors.rb

@@ -3,8 +3,12 @@
 module OctocatalogDiff
   # Contains error classes raised by this gem
   class Errors
-    # Error class for handled configuration file errors
+    # Error classes for handled configuration file errors
     class ConfigurationFileNotFoundError < RuntimeError; end
     class ConfigurationFileContentError < RuntimeError; end
+
+    # Error classes for building catalogs
+    class BootstrapError < RuntimeError; end
+    class CatalogError < RuntimeError; end
   end
 end

lib/octocatalog-diff/util/catalogs.rb

@@ -3,19 +3,15 @@
 require 'json'
 require 'open3'
 require 'yaml'
-require_relative '../catalog-util/bootstrap' # For BootstrapError
 require_relative '../catalog'
+require_relative '../errors'
 require_relative 'parallel'
 
 module OctocatalogDiff
   module Util
     # Helper class to construct catalogs, performing all necessary steps such as
     # bootstrapping directories, installing facts, and running puppet.
     class Catalogs
-      # Exceptions that are anticipated can be caught in the calling class and tested for explicitly in spec tests.
-      class BootstrapError < RuntimeError; end
-      class CatalogError < RuntimeError; end
-
       # Constructor
       # @param options [Hash] Options
       # @param logger [Logger] Logger object
@@ -40,9 +36,6 @@ def bootstrap_then_exit
         OctocatalogDiff::CatalogUtil::Bootstrap.bootstrap_directory_parallelizer(@options, @logger)
         @logger.debug('Success bootstrap_then_exit')
         @logger.info('Successfully completed --bootstrap-then-exit action')
-      rescue OctocatalogDiff::CatalogUtil::Bootstrap::BootstrapError => exc
-        @logger.error("Bootstrap exception: #{exc}")
-        raise BootstrapError, "Bootstrap exception: #{exc}"
       end
 
       private
@@ -121,8 +114,8 @@ def build_catalog_parallelizer
 
         # Things have succeeded if the :to and :from catalogs exist at this point. If not, things have
         # failed, and an exception should be thrown.
-        raise CatalogError, 'One or more catalogs failed to compile.' unless result.key?(:to) && result.key?(:from)
-        result
+        return result if result.key?(:to) && result.key?(:from)
+        raise OctocatalogDiff::Errors::CatalogError, 'One or more catalogs failed to compile.'
       end
 
       # Get catalog compilation tasks.
@@ -201,14 +194,15 @@ def add_parallel_result(result, parallel_catalog_obj, key_task_tuple)
             error_display = catalog.error_message.split("\n").map do |line|
               line.sub(/^Error:/, '[Puppet Error]').gsub(dir_regex, '')
             end.join("\n")
-            raise CatalogError, "Catalog for #{branch} failed to compile due to errors:\n#{error_display}"
+            message = "Catalog for #{branch} failed to compile due to errors:\n#{error_display}"
+            raise OctocatalogDiff::Errors::CatalogError, message
           end
         else
           # Something unhandled went wrong, and an exception was thrown. Reveal a generic message.
           msg = parallel_catalog_obj.exception.message
           message = "Catalog for '#{key}' (#{branch}) failed to compile with #{parallel_catalog_obj.exception.class}: #{msg}"
           message += "\n" + parallel_catalog_obj.exception.backtrace.map { |x| "   #{x}" }.join("\n") if @options[:debug]
-          raise CatalogError, message
+          raise OctocatalogDiff::Errors::CatalogError, message
         end
       end
 

spec/octocatalog-diff/integration/bootstrap_script_spec.rb

@@ -53,7 +53,7 @@
 
     it 'should print error from bootstrap failing' do
       expect(@result[:exception].message).to match(/Catalog for 'to' \(.\) failed to compile with.+::BootstrapError/)
-      expect(@result[:exception].message).to match(/OctocatalogDiff::CatalogUtil::Bootstrap::BootstrapError/)
+      expect(@result[:exception].message).to match(/OctocatalogDiff::Errors::BootstrapError/)
       expect(@result[:exception].message).not_to match(/Could not find class (::)?test/)
     end
 

spec/octocatalog-diff/tests/catalog-util/bootstrap_spec.rb

@@ -3,6 +3,7 @@
 require_relative '../spec_helper'
 
 require OctocatalogDiff::Spec.require_path('/catalog-util/bootstrap')
+require OctocatalogDiff::Spec.require_path('/errors')
 
 require 'fileutils'
 
@@ -25,7 +26,7 @@
       }
       expect do
         OctocatalogDiff::CatalogUtil::Bootstrap.bootstrap_directory_parallelizer(opts, logger)
-      end.to raise_error(OctocatalogDiff::CatalogUtil::Bootstrap::BootstrapError, /Must specify a from-branch/)
+      end.to raise_error(OctocatalogDiff::Errors::BootstrapError, /Must specify a from-branch/)
       expect(logger_str.string).to match(/Must specify a from-branch/)
     end
 
@@ -38,7 +39,7 @@
       }
       expect do
         OctocatalogDiff::CatalogUtil::Bootstrap.bootstrap_directory_parallelizer(opts, logger)
-      end.to raise_error(OctocatalogDiff::CatalogUtil::Bootstrap::BootstrapError, /Must specify a to-branch/)
+      end.to raise_error(OctocatalogDiff::Errors::BootstrapError, /Must specify a to-branch/)
       expect(logger_str.string).to match(/Must specify a to-branch/)
     end
 
@@ -47,7 +48,7 @@
       opts = {}
       expect do
         OctocatalogDiff::CatalogUtil::Bootstrap.bootstrap_directory_parallelizer(opts, logger)
-      end.to raise_error(OctocatalogDiff::CatalogUtil::Bootstrap::BootstrapError, /Specify one or more of/)
+      end.to raise_error(OctocatalogDiff::Errors::BootstrapError, /Specify one or more of/)
       expect(logger_str.string).to match(/Specify one or more of/)
     end
 

spec/octocatalog-diff/tests/cli_spec.rb

@@ -3,6 +3,7 @@
 require_relative 'spec_helper'
 
 require OctocatalogDiff::Spec.require_path('/cli')
+require OctocatalogDiff::Spec.require_path('/errors')
 
 describe OctocatalogDiff::Cli do
   describe '#parse_opts' do
@@ -256,7 +257,7 @@
 
     it 'should fail and exit 1 if BootstrapError occurs' do
       d = double('OctocatalogDiff::Util::Catalogs')
-      allow(d).to receive(:bootstrap_then_exit).and_raise(OctocatalogDiff::Util::Catalogs::BootstrapError, 'hello')
+      allow(d).to receive(:bootstrap_then_exit).and_raise(OctocatalogDiff::Errors::BootstrapError, 'hello')
       logger, logger_str = OctocatalogDiff::Spec.setup_logger
       rc = OctocatalogDiff::Cli.bootstrap_then_exit(logger, d)
       expect(rc).to eq(1)

spec/octocatalog-diff/tests/util/catalogs_spec.rb

@@ -4,6 +4,7 @@
 require_relative '../../mocks/puppetdb'
 
 require OctocatalogDiff::Spec.require_path('/catalog')
+require OctocatalogDiff::Spec.require_path('/errors')
 require OctocatalogDiff::Spec.require_path('/facts')
 require OctocatalogDiff::Spec.require_path('/util/catalogs')
 
@@ -92,7 +93,7 @@ def valid?
       options = @default_options.merge(from_env: 'master')
       logger, logger_string = OctocatalogDiff::Spec.setup_logger
       testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
-      expect { testobj.bootstrap_then_exit }.to raise_error(OctocatalogDiff::Util::Catalogs::BootstrapError)
+      expect { testobj.bootstrap_then_exit }.to raise_error(OctocatalogDiff::Errors::BootstrapError)
       expect(logger_string.string).to match(%r{Specify one or more of --bootstrapped-from-dir / --bootstrapped-to-dir})
     end
 
@@ -105,8 +106,8 @@ def valid?
         options = @default_options.merge(basedir: tmpdir2, bootstrapped_from_dir: tmpdir1, from_env: 'asdfasdfasdf')
         logger, logger_string = OctocatalogDiff::Spec.setup_logger
         testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
-        expect { testobj.bootstrap_then_exit }.to raise_error(OctocatalogDiff::Util::Catalogs::BootstrapError)
-        expect(logger_string.string).to match(/ERROR -- : Bootstrap exception: Failed bootstrap_directory for from_dir/)
+        expect { testobj.bootstrap_then_exit }.to raise_error(OctocatalogDiff::Errors::BootstrapError)
+        expect(logger_string.string).to match(/DEBUG .+ Failed bootstrap from_dir/)
       ensure
         OctocatalogDiff::Spec.clean_up_tmpdir(tmpdir1)
         OctocatalogDiff::Spec.clean_up_tmpdir(tmpdir2)
@@ -121,8 +122,8 @@ def valid?
         options = @default_options.merge(bootstrapped_from_dir: tmpdir1, from_env: 'asdfasdfasdf')
         logger, logger_string = OctocatalogDiff::Spec.setup_logger
         testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
-        expect { testobj.bootstrap_then_exit }.to raise_error(OctocatalogDiff::Util::Catalogs::BootstrapError)
-        expect(logger_string.string).to match(/ERROR -- : Bootstrap exception: Failed bootstrap_directory for from_dir/)
+        expect { testobj.bootstrap_then_exit }.to raise_error(OctocatalogDiff::Errors::BootstrapError)
+        expect(logger_string.string).to match(/DEBUG .+ Failed bootstrap from_dir/)
       ensure
         OctocatalogDiff::Spec.clean_up_tmpdir(tmpdir1)
       end
@@ -143,7 +144,7 @@ def valid?
             @bootstrap_error_message = nil
             ENV['PATH'] = '/usr/sbin:/sbin:/usr/bin:/bin:/usr/local/bin:/usr/local/sbin'
             testobj.bootstrap_then_exit
-          rescue OctocatalogDiff::Util::Catalogs::BootstrapError => exc
+          rescue OctocatalogDiff::Errors::BootstrapError => exc
             @bootstrap_error_message = "BootstrapError #{exc}: #{logger_string.string}"
           ensure
             ENV['PATH'] = path_save
@@ -197,7 +198,7 @@ def valid?
         logger, _logger_string = OctocatalogDiff::Spec.setup_logger
         testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
         re = %r{ENC.*/asdkfjlfjkalksdfads wasn't found}
-        expect { testobj.catalogs }.to raise_error(OctocatalogDiff::Util::Catalogs::CatalogError, re)
+        expect { testobj.catalogs }.to raise_error(OctocatalogDiff::Errors::CatalogError, re)
       end
     end
   end
@@ -210,7 +211,7 @@ def valid?
         logger, _logger_string = OctocatalogDiff::Spec.setup_logger
         testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
         re = %r{hiera.yaml.*/asdkfjlfjkalksdfads\) wasn't found}
-        expect { testobj.catalogs }.to raise_error(OctocatalogDiff::Util::Catalogs::CatalogError, re)
+        expect { testobj.catalogs }.to raise_error(OctocatalogDiff::Errors::CatalogError, re)
       end
     end
   end
@@ -343,7 +344,7 @@ def valid?
       answer = Regexp.new(lines.join('(.|\n)*'))
       expect do
         testobj.send(:add_parallel_result, result, pcr, key_task_tuple)
-      end.to raise_error(OctocatalogDiff::Util::Catalogs::CatalogError, answer)
+      end.to raise_error(OctocatalogDiff::Errors::CatalogError, answer)
 
       expect(logger_str.string).to eq('')
     end