crimdon / SQLToolkit

An Extension for Azure DevOps Server
MIT License
17 stars 14 forks source link

Run Sql Scripts error information lacking #72

Closed tophallen closed 4 years ago

tophallen commented 5 years ago

When running many SQL scripts in the Run Sql Scripts task there is a lack of error information.

[error]Error running SQL script

all it does is write the offending script and say it failed.

Most other tasks in this library have an appropriate catch block:

Try {
} Catch {
    Write-Host "Error running SQL script: $_" -ForegroundColor Red
    throw $_
}

But this task does nothing to identify the actual error that occurred:

Try {
  $SqlCmd.CommandText = $_.Trim(); 
  $reader = $SqlCmd.ExecuteNonQuery(); 
} Catch {
  switch ($continueAfterError) {
    $true {Write-Host "$($SqlCmd.CommandText) resulted in an error"; }
    $false {
      Write-Host "$($SqlCmd.CommandText) resulted in an error"; 
      throw "Fatal error ocurred.";
    }
  }
} 

Simple proposal, we need to capture the error that occurred on each script, even if continuing after error.

Try {
  $SqlCmd.CommandText = $_.Trim(); 
  $reader = $SqlCmd.ExecuteNonQuery(); 
} Catch {
  switch ($continueAfterError) {
    $true {Write-Host "$($SqlCmd.CommandText) resulted in an error $_"; }
    $false {
      Write-Host "$($SqlCmd.CommandText) resulted in an error $_" -ForegroundColor Red; 
      throw $_;
    }
  }
} 
tophallen commented 5 years ago

I have the fix for this in my fork and would be happy to make a pull request for this fix. https://github.com/crimdon/SQLToolkit/compare/master...tophallen:master

The other change in that PR solves the issue of when a script file for some reason starts with a GO statement. Since SSMS handles that scenario, it would make sense that this extension should too, even if it shouldn't be there.

crimdon commented 5 years ago

Hi Allan

I don't mind putting the try catch in but I'm going away for 2 weeks so I'd rather leave it till I get back. Also the last time I implemented your PR for the GO issue, I was forced to revert it as a load of people complained it broke their builds.

Andrew

tophallen commented 5 years ago

Interesting, I had tested it on a different scenarios - I'll isolate the try/catch logging and make a PR.

tophallen commented 5 years ago

https://github.com/crimdon/SQLToolkit/compare/master...tophallen:master

If that looks agreeable, I'll create the PR

crimdon commented 5 years ago

Hi there...sorry for delay in replying but I've just got back to work this week after a break. Yeah that looks fine to me

DuckoDeath commented 4 years ago

any chance this can get released?

crimdon commented 4 years ago

Hi there, I need to check this as I thought I had. Will get back to you

crimdon commented 4 years ago

Thats released now