(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

lib/puppet_x/sqlserver/sql_connection.rb

@@ -9,12 +9,12 @@ def open_and_run_command(query, config)
           open(config)
           execute(query)
         rescue win32_exception => e
-          return ResultOutput.new(true, e.message)
+          return ResultOutput.new(true, e.message, @connection)
         ensure
           close
         end
 
-        ResultOutput.new(false, nil)
+        ResultOutput.new(false, nil, @connection)
       end
 
       private
@@ -90,25 +90,20 @@ 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 = connection.Errors(0).Description if connection && !connection.Errors.count == 0
+
+        @error_message ||= error_message
       end
 
       def has_errors
         @exitstatus != 0
       end
-
-      private
-      def parse_for_error(result)
-        match = result.match(/SQL Server\n\s*(.*)/i)
-        match[1] unless match == nil
-      end
     end
   end
 end

spec/unit/puppet_x/sql_connection_spec.rb

@@ -8,8 +8,12 @@
 
   def stub_connection
     @connection = mock()
+    error = mock()
+    error.stubs(:Description).returns("SQL Connection error has happened")
+    error.stubs(:count).returns(1)
     subject.stubs(:create_connection).returns(@connection)
     @connection.stubs(:State).returns(PuppetX::Sqlserver::CONNECTION_CLOSED)
+    @connection.stubs(:Errors).returns(error)
     subject.stubs(:win32_exception).returns(Exception)
   end
 
@@ -20,11 +24,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"))
+        subject.stubs(:execute).raises(Exception.new("An error has happened"))
         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('An error has happened')
         }.to_not raise_error(Exception)
 
       end
@@ -116,12 +120,12 @@ def stub_connection
       it {
         subject.stubs(:win32_exception).returns(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(:execute).with('SELECT * FROM sys.databases').raises(Exception.new("Invalid syntax provider"))
         subject.expects(:close).once
         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('Invalid syntax provider')
       }
     end
     context 'open connection failure' do
@@ -132,7 +136,7 @@ def stub_connection
         expect {
           result = subject.open_and_run_command('whacka whacka whacka', config)
           expect(result.exitstatus).to eq(1)
-          expect(result.error_message).to eq 'ConnectionFailed'
+          expect(result.error_message).to eq "SQL Server\n ConnectionFailed"
         }.to_not raise_error(Exception)
       }
     end