Move PuppetDB errors to error class

Author: Kevin Paulisse

lib/octocatalog-diff/catalog/puppetdb.rb

@@ -3,6 +3,7 @@
 require 'json'
 require 'stringio'
 
+require_relative '../errors'
 require_relative '../puppetdb'
 
 module OctocatalogDiff
@@ -67,11 +68,11 @@ def fetch_catalog(logger)
             # Set the other variables
             @catalog_json = ::JSON.generate(@catalog)
             @error_message = nil
-          rescue OctocatalogDiff::PuppetDB::ConnectionError => exc
+          rescue OctocatalogDiff::Errors::PuppetDBConnectionError => exc
             @error_message = "Catalog retrieval failed (#{exc.class}) (#{exc.message})"
-          rescue OctocatalogDiff::PuppetDB::NotFoundError => exc
+          rescue OctocatalogDiff::Errors::PuppetDBNodeNotFoundError => exc
             @error_message = "Node #{node} not found in PuppetDB (#{exc.message})"
-          rescue OctocatalogDiff::PuppetDB::PuppetDBError => exc
+          rescue OctocatalogDiff::Errors::PuppetDBGenericError => exc
             @error_message = "Catalog retrieval failed for node #{node} from PuppetDB (#{exc.message})"
           rescue ::JSON::GeneratorError => exc
             @error_message = "Failed to generate result from PuppetDB as JSON (#{exc.message})"

lib/octocatalog-diff/errors.rb

@@ -14,5 +14,10 @@ class CatalogError < RuntimeError; end
     # Error classes for retrieving facts
     class FactSourceError < RuntimeError; end
     class FactRetrievalError < RuntimeError; end
+
+    # Errors for PuppetDB
+    class PuppetDBNodeNotFoundError < RuntimeError; end
+    class PuppetDBGenericError < RuntimeError; end
+    class PuppetDBConnectionError < RuntimeError; end
   end
 end

lib/octocatalog-diff/facts/puppetdb.rb

@@ -50,13 +50,13 @@ def self.fact_retriever(options = {}, node)
             obj_to_return = { 'name' => node, 'values' => {} }
             facts.each { |k, v| obj_to_return['values'][k.sub(/^::/, '')] = v }
             break # Not return, to avoid LocalJumpError in Ruby 2.2
-          rescue OctocatalogDiff::PuppetDB::ConnectionError => exc
+          rescue OctocatalogDiff::Errors::PuppetDBConnectionError => exc
             exception_class = OctocatalogDiff::Errors::FactSourceError
             exception_message = "Fact retrieval failed (#{exc.class}) (#{exc.message})"
-          rescue OctocatalogDiff::PuppetDB::NotFoundError => exc
+          rescue OctocatalogDiff::Errors::PuppetDBNodeNotFoundError => exc
             exception_class = OctocatalogDiff::Errors::FactRetrievalError
             exception_message = "Node #{node} not found in PuppetDB (#{exc.message})"
-          rescue OctocatalogDiff::PuppetDB::PuppetDBError => exc
+          rescue OctocatalogDiff::Errors::PuppetDBGenericError => exc
             exception_class = OctocatalogDiff::Errors::FactRetrievalError
             exception_message = "Fact retrieval failed for node #{node} from PuppetDB (#{exc.message})"
           end

lib/octocatalog-diff/puppetdb.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative 'errors'
 require_relative 'util/httparty'
 
 require 'uri'
@@ -14,11 +15,6 @@
 module OctocatalogDiff
   # A standard way to connect to PuppetDB from the various scripts in this repository.
   class PuppetDB
-    # This class raises errors for certain handled problems
-    class NotFoundError < RuntimeError; end
-    class PuppetDBError < RuntimeError; end
-    class ConnectionError < RuntimeError; end
-
     # Allow connections to be read (used in tests for now)
     attr_reader :connections
 
@@ -87,7 +83,7 @@ def initialize(options = {})
     def get(path)
       _get(path)
     rescue Net::OpenTimeout, Errno::ECONNREFUSED => exc
-      raise ConnectionError, "#{exc.class} connecting to PuppetDB (need VPN on?): #{exc.message}"
+      raise OctocatalogDiff::Errors::PuppetDBConnectionError, "#{exc.class} connecting to PuppetDB (need VPN on?): #{exc.message}"
     end
 
     private
@@ -120,16 +116,16 @@ def _get(path)
 
           # Handle all non-200's from PuppetDB
           unless response[:code] == 200
-            raise NotFoundError, "404 - #{response[:error]}" if response[:code] == 404
-            raise PuppetDBError, "#{response[:code]} - #{response[:error]}"
+            raise OctocatalogDiff::Errors::PuppetDBNodeNotFoundError, "404 - #{response[:error]}" if response[:code] == 404
+            raise OctocatalogDiff::Errors::PuppetDBGenericError, "#{response[:code]} - #{response[:error]}"
           end
 
           # PuppetDB can return 'Not Found' as a string with a 200 response code
           raise NotFoundError, '404 - Not Found' if response[:body] == 'Not Found'
 
           # PuppetDB can also return an error message in a 200; we'll call this a 500
           if response.key?(:error)
-            raise PuppetDBError, "500 - #{response[:error]}"
+            raise OctocatalogDiff::Errors::PuppetDBGenericError, "500 - #{response[:error]}"
           end
 
           # If we get here without raising an error, it will fall out of the begin/rescue

spec/octocatalog-diff/mocks/puppetdb.rb

@@ -2,6 +2,7 @@
 
 require_relative '../tests/spec_helper'
 
+require OctocatalogDiff::Spec.require_path('/errors')
 require OctocatalogDiff::Spec.require_path('/puppetdb')
 
 module OctocatalogDiff
@@ -59,7 +60,7 @@ def override_facts(hostname)
       # @return [String] JSON catalog
       def catalog(hostname)
         fixture_file = OctocatalogDiff::Spec.fixture_path(File.join('catalogs', "#{hostname}.json"))
-        raise OctocatalogDiff::PuppetDB::NotFoundError, '404 - Not Found' unless File.file?(fixture_file)
+        raise OctocatalogDiff::Errors::PuppetDBNodeNotFoundError, '404 - Not Found' unless File.file?(fixture_file)
         JSON.parse(File.read(fixture_file))
       end
     end

spec/octocatalog-diff/tests/catalog/puppetdb_spec.rb

@@ -8,6 +8,7 @@
 require OctocatalogDiff::Spec.require_path('/catalog')
 require OctocatalogDiff::Spec.require_path('/catalog/puppetdb')
 require OctocatalogDiff::Spec.require_path('/catalog-util/builddir')
+require OctocatalogDiff::Spec.require_path('/errors')
 
 describe OctocatalogDiff::Catalog::PuppetDB do
   context 'with working catalog' do
@@ -106,16 +107,16 @@
 
       it 'should set error message for connection error' do
         puppetdb = double('OctocatalogDiff::PuppetDB')
-        allow(puppetdb).to receive(:get).and_raise(OctocatalogDiff::PuppetDB::ConnectionError, 'test')
+        allow(puppetdb).to receive(:get).and_raise(OctocatalogDiff::Errors::PuppetDBConnectionError, 'test')
         allow(OctocatalogDiff::PuppetDB).to receive(:new).and_return(puppetdb)
 
         @obj.send(:fetch_catalog, @logger)
-        expect(@obj.error_message).to match(/Catalog retrieval failed \(.*::ConnectionError\) \(test\)/)
+        expect(@obj.error_message).to match(/Catalog retrieval failed \(.*::PuppetDBConnectionError\) \(test\)/)
       end
 
       it 'should set error message for not found error' do
         puppetdb = double('OctocatalogDiff::PuppetDB')
-        allow(puppetdb).to receive(:get).and_raise(OctocatalogDiff::PuppetDB::NotFoundError, 'test')
+        allow(puppetdb).to receive(:get).and_raise(OctocatalogDiff::Errors::PuppetDBNodeNotFoundError, 'test')
         allow(OctocatalogDiff::PuppetDB).to receive(:new).and_return(puppetdb)
 
         @obj.send(:fetch_catalog, @logger)
@@ -124,7 +125,7 @@
 
       it 'should set error message for puppetdb error' do
         puppetdb = double('OctocatalogDiff::PuppetDB')
-        allow(puppetdb).to receive(:get).and_raise(OctocatalogDiff::PuppetDB::PuppetDBError, 'test')
+        allow(puppetdb).to receive(:get).and_raise(OctocatalogDiff::Errors::PuppetDBGenericError, 'test')
         allow(OctocatalogDiff::PuppetDB).to receive(:new).and_return(puppetdb)
 
         @obj.send(:fetch_catalog, @logger)

spec/octocatalog-diff/tests/facts/puppetdb_spec.rb

@@ -80,27 +80,27 @@
       let(:opts) { { puppetdb_url: 'https://mocked-puppetdb.somedomain.xyz:8081', node: 'valid-facts' } }
       let(:node) { 'valid-facts' }
 
-      it 'should handle OctocatalogDiff::PuppetDB::ConnectionError' do
+      it 'should handle OctocatalogDiff::Errors::PuppetDBConnectionError' do
         obj = double('OctocatalogDiff::PuppetDB')
-        allow(obj).to receive(:get).and_raise(OctocatalogDiff::PuppetDB::ConnectionError, 'test message')
+        allow(obj).to receive(:get).and_raise(OctocatalogDiff::Errors::PuppetDBConnectionError, 'test message')
         allow(OctocatalogDiff::PuppetDB).to receive(:new) { |*_arg| obj }
         expect do
           OctocatalogDiff::Facts::PuppetDB.fact_retriever(opts, node)
         end.to raise_error(OctocatalogDiff::Errors::FactSourceError, /Fact retrieval failed \(.*ConnectionError\) \(test/)
       end
 
-      it 'should handle OctocatalogDiff::PuppetDB::NotFoundError' do
+      it 'should handle OctocatalogDiff::Errors::PuppetDBNodeNotFoundError' do
         obj = double('OctocatalogDiff::PuppetDB')
-        allow(obj).to receive(:get).and_raise(OctocatalogDiff::PuppetDB::NotFoundError, 'test message')
+        allow(obj).to receive(:get).and_raise(OctocatalogDiff::Errors::PuppetDBNodeNotFoundError, 'test message')
         allow(OctocatalogDiff::PuppetDB).to receive(:new) { |*_arg| obj }
         expect do
           OctocatalogDiff::Facts::PuppetDB.fact_retriever(opts, node)
         end.to raise_error(OctocatalogDiff::Errors::FactRetrievalError, /Node valid-facts not found in PuppetDB \(test/)
       end
 
-      it 'should handle OctocatalogDiff::PuppetDB::PuppetDBError' do
+      it 'should handle OctocatalogDiff::Errors::PuppetDBGenericError' do
         obj = double('OctocatalogDiff::PuppetDB')
-        allow(obj).to receive(:get).and_raise(OctocatalogDiff::PuppetDB::PuppetDBError, 'test message')
+        allow(obj).to receive(:get).and_raise(OctocatalogDiff::Errors::PuppetDBGenericError, 'test message')
         allow(OctocatalogDiff::PuppetDB).to receive(:new) { |*_arg| obj }
         expect do
           OctocatalogDiff::Facts::PuppetDB.fact_retriever(opts, node)

spec/octocatalog-diff/tests/puppetdb_spec.rb

@@ -2,6 +2,7 @@
 
 # Test the OctocatalogDiff::PuppetDB library
 require_relative 'spec_helper'
+require OctocatalogDiff::Spec.require_path('/errors')
 require OctocatalogDiff::Spec.require_path('/puppetdb')
 require 'uri'
 
@@ -174,7 +175,7 @@ def ssl_test(server_opts, opts = {})
         puppetdb_url: 'http://127.0.0.1:1'
       }
       testobj = OctocatalogDiff::PuppetDB.new(opts)
-      expect { testobj.get('/foo') }.to raise_error(OctocatalogDiff::PuppetDB::ConnectionError)
+      expect { testobj.get('/foo') }.to raise_error(OctocatalogDiff::Errors::PuppetDBConnectionError)
     end
 
     it 'should timeout correctly' do
@@ -187,7 +188,7 @@ def ssl_test(server_opts, opts = {})
       }
       testobj = OctocatalogDiff::PuppetDB.new(opts)
       time_begin = Time.now.to_i
-      expect { testobj.get('/foo') }.to raise_error(OctocatalogDiff::PuppetDB::ConnectionError)
+      expect { testobj.get('/foo') }.to raise_error(OctocatalogDiff::Errors::PuppetDBConnectionError)
       time_end = Time.now.to_i
       expect(time_end - time_begin).to be <= 5
     end
@@ -463,17 +464,17 @@ def ssl_test(server_opts, opts = {})
 
     it 'should handle 404 responses' do
       allow(OctocatalogDiff::Util::HTTParty).to receive(:get).and_return(code: 404, error: 'oh noez')
-      expect { @testobj.send(:get, '/foo') }.to raise_error(OctocatalogDiff::PuppetDB::NotFoundError, /oh noez/)
+      expect { @testobj.send(:get, '/foo') }.to raise_error(OctocatalogDiff::Errors::PuppetDBNodeNotFoundError, /oh noez/)
     end
 
     it 'should handle !=200, !=404 responses' do
       allow(OctocatalogDiff::Util::HTTParty).to receive(:get).and_return(code: 499, error: 'oh noez')
-      expect { @testobj.send(:get, '/foo') }.to raise_error(OctocatalogDiff::PuppetDB::PuppetDBError, /oh noez/)
+      expect { @testobj.send(:get, '/foo') }.to raise_error(OctocatalogDiff::Errors::PuppetDBGenericError, /oh noez/)
     end
 
     it 'should handle responses with :error' do
       allow(OctocatalogDiff::Util::HTTParty).to receive(:get).and_return(code: 200, error: 'oh noez')
-      expect { @testobj.send(:get, '/foo') }.to raise_error(OctocatalogDiff::PuppetDB::PuppetDBError, /500 - oh noez/)
+      expect { @testobj.send(:get, '/foo') }.to raise_error(OctocatalogDiff::Errors::PuppetDBGenericError, /500 - oh noez/)
     end
 
     it 'should handle unparseable responses' do