Restore-DbaDatabase\Test-DbaBackupInformation - EnableException doesn't change warning to error if the files are already owned by another database on the instance

jpomfret commented 5 months ago

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

I got this warning - even when specifying -EnableException it doesn't change to throw an error and therefore can't be trapped

Steps to Reproduce

Need to already have a pubs database existing and then try and restore a second with a new name


$restoreParams = @{
    SqlInstance = $primary
    DatabaseName = ('{0}_jp' -f $psitem)
    Path = ('{0}/{1}' -f $backupPath, $db)
    RestoreTime = $restoreToTime
    WithReplace = $true
    EnableException = $true
}
Restore-DbaDatabase @restoreParams -verbose

Please confirm that you are running the most recent version of dbatools

2.1.5

Other details or mentions

No response

What PowerShell host was used when producing this error

VS Code (terminal), VS Code (integrated terminal)

PowerShell Host Version

Name                           Value
----                           -----
PSVersion                      7.3.11
PSEdition                      Core
GitCommitId                    7.3.11
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

SQL Server Edition and Build number

.NET Framework Version

.NET 7.0.15

andreasjordan commented 4 months ago

Hi Jess!

All three warning messages are generated by "Write-Message". To be able to raise an exception when using EnableException, we would have to change some or all of them to "Stop-Function".

I would suggest to change line 768 of Restore-DbaDatabase (Write-Message -Level Warning -Message "Database $DbUnverified failed testing, skipping") to Stop-Function -Message "Database $DbUnverified failed testing, skipping".

We would have to think about a good message, as the current message is ok for EnableException:$false, but maybe not the best for EnabledException:$true.

Here is a screenshot from my tests:

andreasjordan commented 3 months ago

@jpomfret Can you help with a good message? I can do the coding and open the pull request.

jpomfret commented 3 months ago

Hey @andreasjordan - what about Database $DbUnverified unable to be restored, it would be nice to also collect why it couldn't be restored but I'm not sure how easy that is looking at the code

andreasjordan commented 3 months ago

The details are still shown in the warning, so I changed the message to Database $DbUnverified unable to be restored, see warnings for details. What do you think?

jpomfret commented 3 months ago

Yeah I like that - sounds good!

dataplat / dbatools