(MODULES-6281) Return Errors from T-SQL

The sqlserver_tsql resource does not handle errors returned from T-SQL
statements properly. The errors come back from the query but unless the
phrase 'SQL Server' was included in the error text, the error was not
then passed back to the user/logs. This change ensure that errors
returned are passed back properly by inspecting the Errors property of
the ADO object used to run the query. Note that this may not behave as
expected if the error that occurrs is of sufficently low severity that a
rowset, even if empty, is also returned. If a rowset is returned by the
ADO object, the object will not populate the Errors property, and there
is no way to tell a low severity error occurred. For instance 'Select
1/0' returns a low severity division by zero error, and an empty rowset.
Because the empty rowset exists, the error cannot be seen. It is up to
the user to ensure that the errors they want to catch are of sufficienty
severity that execution of the query is halted.

Author: RandomNoun7

CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 - During acceptance testing, only execute master provisioning steps if there is
   a master in the hosts array.
 - Stop running ```gem update bundler``` during Travis runs. ([MODULES-6339](https://tickets.puppetlabs.com/browse/MODULES6339))
+- The `sqlserver_tsql` resource now returns errors from sql queries properly. ([MODULES-6281](https://tickets.puppetlabs.com/browse/MODULES-6281))
 
 ## [2.1.0] - 2017-12-8
 

lib/puppet_x/sqlserver/sql_connection.rb

@@ -9,12 +9,15 @@ def open_and_run_command(query, config)
           open(config)
           execute(query)
         rescue win32_exception => e
-          return ResultOutput.new(true, e.message)
+          # require 'pry'; binding.pry;
+          return ResultOutput.new(true, e.message, @connection)
         ensure
           close
         end
 
-        ResultOutput.new(false, nil)
+        # require 'pry'; binding.pry;
+
+        ResultOutput.new(false, nil, @connection)
       end
 
       private
@@ -90,24 +93,25 @@ def win32_exception
     end
 
     class ResultOutput
-      attr_reader :exitstatus, :error_message, :raw_error_message
 
-      def initialize(has_errors, error_message)
+      attr_reader :exitstatus, :error_message
+
+      def initialize(has_errors, error_message, connection)
         @exitstatus = has_errors ? 1 : 0
-        if error_message
-          @raw_error_message = error_message
-          @error_message = parse_for_error(error_message)
-        end
+
+        @error_message = (extract_messages(connection) || error_message)
       end
 
-      def has_errors
-        @exitstatus != 0
+      def extract_messages(connection)
+        return nil if connection.nil? || connection.Errors.count == 0
+
+        error_count = connection.Errors.count - 1
+        
+        ((0..error_count).map { |i| connection.Errors(i).Description.to_s}).join("\n")
       end
 
-      private
-      def parse_for_error(result)
-        match = result.match(/SQL Server\n\s*(.*)/i)
-        match[1] unless match == nil
+      def has_errors
+        @exitstatus != 0
       end
     end
   end

spec/acceptance/sqlserver_tsql_spec.rb

@@ -191,7 +191,7 @@ def ensure_sqlserver_database(db_name, ensure_val = 'present')
       }
       MANIFEST
       execute_manifest(pp, {:acceptable_exit_codes => [0,1]}) do |r|
-        expect(r.stderr).to match(/Error/i)
+        expect(r.stderr).to match(/Incorrect syntax/i)
       end
     end
 
@@ -210,7 +210,7 @@ def ensure_sqlserver_database(db_name, ensure_val = 'present')
       }
       MANIFEST
       execute_manifest(pp, {:acceptable_exit_codes => [0,1]}) do |r|
-        expect(r.stderr).to match(/Error/i)
+        expect(r.stderr).to match(/Non-Existing-Database/i)
       end
     end
   end

spec/unit/puppet_x/sql_connection_spec.rb

@@ -8,8 +8,13 @@
 
   def stub_connection
     @connection = mock()
+    error_mock = mock()
+    error_mock.stubs(:Description).returns("SQL Error in Connection")
+    error_mock.stubs(:count).returns(0)
+
     subject.stubs(:create_connection).returns(@connection)
     @connection.stubs(:State).returns(PuppetX::Sqlserver::CONNECTION_CLOSED)
+    @connection.stubs(:Errors).returns(error_mock)
     subject.stubs(:win32_exception).returns(Exception)
   end
 
@@ -20,11 +25,11 @@ def stub_connection
         @connection.stubs(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.;DataTypeComptibility=80;User ID=sa;Password=Pupp3t1@')
       end
       it 'should not raise an error but populate has_errors with message' do
-        subject.stubs(:execute).raises(Exception.new("SQL Server\n error has happened"))
+        @connection.Errors.stubs(:count).returns(1)
         expect {
           result = subject.open_and_run_command('whacka whacka whacka', config)
           expect(result.exitstatus).to eq(1)
-          expect(result.error_message).to eq('error has happened')
+          expect(result.error_message).to eq('SQL Error in Connection')
         }.to_not raise_error(Exception)
 
       end
@@ -114,20 +119,20 @@ def stub_connection
     end
     context 'return result with errors' do
       it {
-        subject.stubs(:win32_exception).returns(Exception)
+        stub_connection
+        @connection.Errors.stubs(:count).returns(1)
+        @connection.stubs(:Execute).raises(Exception)
         subject.expects(:open).with({:admin_user => 'sa', :admin_pass => 'Pupp3t1@', :instance_name => 'MSSQLSERVER'})
-        subject.expects(:execute).with('SELECT * FROM sys.databases').raises(Exception.new("SQL Server\ninvalid syntax provider"))
         subject.expects(:close).once
-        result =
-          subject.open_and_run_command('SELECT * FROM sys.databases', config)
+        result = subject.open_and_run_command('SELECT * FROM sys.databases', config)
         expect(result.exitstatus).to eq(1)
-        expect(result.error_message).to eq('invalid syntax provider')
+        expect(result.error_message).to eq('SQL Error in Connection')
       }
     end
     context 'open connection failure' do
       it {
         stub_connection
-        err_message = "SQL Server\n ConnectionFailed"
+        err_message = "ConnectionFailed"
         @connection.stubs(:Open).raises(Exception.new(err_message))
         expect {
           result = subject.open_and_run_command('whacka whacka whacka', config)