Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixes and a new Force option for Remove-SqlDatabase #9

Closed
wants to merge 4 commits into from

Conversation

derfsplat
Copy link
Contributor

No description provided.

If we fail to connect to the provide SQL Instance (e.g. .\SQLEXPRESS), this method will fail silently with no error information returned to the caller.
This is useful for removing a database that was created in multi-user mode as there may be cached conenctions to SQL still open after running integration tests.
@derfsplat
Copy link
Contributor Author

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

Yeah, they should probably all be Write-Error .. .though we prob only need two in there total I think.

If for some reason RollbackTransaction throws, then you'll never get the 3rd line anyhow...

Write-Error '[ERROR]: DB update completed with errors.`n$($_.ToString())

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

You know what though, I think that I didn't Write-Error here on purpose... any time an exception is caught, it automatically makes its way into the global $Error collection...

And at the end the output is rounded up for display by Resolve-Error

Adding additional Write-Error calls will artificially inflate the $Error collection.

Could you double check this in your tests.

Start with an $Error.Clear(), then do what yo'ure doing... the failure should be parsed at the top level by Psake.

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

And you should also see it inside $Error

@derfsplat
Copy link
Contributor Author

Errors thrown here were not being added to the $Error collection- I tested this. If I do, say, $Error[0].ToString, I get "cannot index into a null array". That's why I did $_.ToString() - not sure what's up with this.

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

Hmm... let me run a quick test of something here. Maybe catch eats them?

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

Nopers, catch shouldn't eat them... I didn't think it did, but you can verify with this.

function Test
{
  $Error.Clear()
  try
  {
    throw 'Foo'
  }
  catch
  {
    Write-Host 'foo'
  }

  Write-Host "There are $($Error.Count) local errors"
}

@derfsplat
Copy link
Contributor Author

Could be related to the fact that this is a module? When I execute and disable access for the connecting user (sa), the error is swallowed and never reported. My first thought was to check the $error collection, but I got the error above (cannot index into a null array).

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

I did run across a scoping issue before.. where the Powershell guys made a bit of a strange call

http://stackoverflow.com/questions/11442023/powershell-error-object-not-immediately-populating-inside-psm1-module

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

Trying to remember.. but perhaps its the TODO that I have there that needs to be addressed. There must have been some reason that I didn't just turn it into a throw though.

That catch block was definitely hit when you added the Write-Host logging in? ... because I'm not sure how a caught exception wouldn't end up in $Error -- that part is not adding up for me.

@derfsplat
Copy link
Contributor Author

Yes it was. I can repro and send you screen shots??? I'm not sure how I can test for the case I ran into bc it involved specifically the SqlException of login failed in that specific section of script. The error was swallowed and the rest of the scripts carried on merrily.

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

I just repro'd a fail locally (outside of Psake) with what's in there.

± $Error.Clear();$Error
eps@epshq-devimage: C:\s\M\tools [ps3 +1 ~0 -0 !]                                 ▶▶▶▶▶▶▶▶▶▶
± Invoke-SqlFileSmo -Path c:\source\midori\tools\foo.sql -UserName sa -Password booboo -Database '.\SQLEXPRESS'


[START] - Running c:\source\midori\tools\foo.sql against .\SQLEXPRESS on db master
eps@epshq-devimage: C:\s\M\tools [ps3 +1 ~0 -0 !]                                 ▶▶▶▶▶▶▶▶▶▶
± $Error
Exception calling "ExecuteNonQuery" with "1" argument(s): "Failed to connect to server .\SQLEXPRESS."
At C:\source\Midori\tools\Sql.psm1:977 char:5
+     $affected = $server.ConnectionContext.ExecuteNonQuery([IO.File]::ReadAllText ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : ConnectionFailureException

The exception definitely makes it into the $Error collection.

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

If I simulate how Psake dumps the output (basically by Resolve-Error $Error) I also get the failures.

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

Ok -- I think maybe the best thing here is to rethrow in the catch, rather than logging anything. I don't think there's a problem with the Exception ending up in $Error, but because its being caught, that's why you're seeing roughly on error resume next type behavior.

Which is I think your primary issue here... that the script continues to execute despite having some sort of SQL connectivity problem.

You can test this pretty easy

function SqlTest
{

  Invoke-SqlFileSmo -Path c:\source\midori\tools\foo.sql -UserName sa -Password booboo -Database '.\SQLEXPRESS'

  Write-Host 'The script proceeded'
}

In the current script, you get The script proceeded while with the addition of throw, you get the error in the console / output, theres only one instance in $Error (where there will be more if you use Write-Error) and finally still executes properly. The try / catch / finally is roughly an implementation of IDisposable since PS has no using

I will push up the rethrow tweak -- you can kill those changesets from your pull and I'll pull in the other little tweaks.

@Iristyle
Copy link
Owner

Iristyle commented Oct 3, 2012

Nvm.. I just cherry picked the 2 changesets in.

@Iristyle Iristyle closed this Oct 3, 2012
@derfsplat
Copy link
Contributor Author

Below is the output from the build script with the "sa" user disabled. Note how the script clearly enters the catch block, does not throw an error, and then continues. The build finishes successfully. Without an explicit output in the catch block, you'd never know there was an error. Along with trying to print the $Error[0] object, I also tried an explicit "throw"- neither resulted in an exception.

FYI: Changing the Write-Host to Write-Error will throw the error, but you have to explicitly Write-Error.
I just wanted to repro so you didn't think I was crazy ;0.

Generating post script for temporary database
Service [MSSQL$SQLEXPRESS] is running
Creating database [E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Database\FDFGBZgDuK.mdf] with service [MSSQL$SQLEXPRESS]
Added PRIMARY filegroup [E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Database\FDFGBZgDuK.mdf] to database...
Customized database using callback
Created database, executing SQL to setup tables and friends...
Shrinking [FDFGBZgDuK]...
Adding test data to test db

[START] - Running E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Database\TestData.sql against .\SQLEXPRESS on db FDFGBZgDuK
Setting initial catalog to FDFGBZgDuK
DB update completed with errors.
Failed to connect to server .\SQLEXPRESS.
Generating [E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Database\FDFGBZgDuK.bak] for [FDFGBZgDuK]
Restoring [E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Database\FDFGBZgDuK.bak] to [DBFC001]at [E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Database\DBFC001.mdf]
Psake: Executing Test at 10/03/2012 16:41:57
Wrote XUnit project for 6 asms to C:\Users\fjohanns\AppData\Local\Temp\tmp55B9.tmp.xunit
Invoking XUnit against C:\Users\fjohanns\AppData\Local\Temp\tmp55B9.tmp.xunit

xUnit.net console test runner (64-bit .NET 4.0.30319.17626)
Copyright (C) 2007-11 Microsoft Corporation.

xunit.dll: Version 1.9.1.1600
Test assembly: E:\Development\Source\Hg\fieldcomm\build\BuildArtifacts\Clients\Field-Comm\FieldComm.API.Client.Tests.Integration.dll

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants