Switch-D365ActiveDatabase should generate Error instead of Warning

[valerymoskalenko]

I run Switch-D365ActiveDatabase on the environment that already has the AxDB_original database. Switch-D365ActiveDatabase just give me a warning message that I need to remove
AxDB_original database.

I think it shouldn't be a warning. It should an Error message. Because database switching has been failed. And I should re-execute the Switch-D365ActiveDatabase after error fixing

[Splaxi]

Thanks for the suggestion 😀

The current implementation is the preferred approach, so we don't have errors, exceptions and other things contaminating the console. We actually don't want to throw exceptions, because it breaks execution, as you expect. This can be hard / difficult to work around when first starting on that path, but when you first get used to it, you don't wanna go back.

I would propose that you do the following:

1. Simple execute Remove-D365Database, if you would have done that anyways. It shouldn't break if the database doesn't exist, so it should be safe to execute.

2. Use Get-D365Database to determine that your environment is in the expected state, before running the Switch-D365Database. If found, remove it or throw an error on your own.

[valerymoskalenko]

Thanks for the details!

I can't agree with you. I think you have the wrong strategy. This behavior may get into the data loss or wrong data on the server and nobody mentions it.
Well, It's like, the holy war on "Should web service generates 400 response code or 200 code only" or "Linux vs Windows".
But this is your project - you are the architector.

Could you just add MORE RED colors on this message?

[devax]

@valerymoskalenko Just add the additional parameter -WarningAction Stop to your command, similar to this: Switch-D365ActiveDatabase -WarningAction Stop. This will basically treat warnings like exceptions, so that you can surround that line with try/catch and that way "catch" the warning.
Here is a small example that illustrates how a warning will terminate the program execution and can be handled in a try/catch block:

function Test-MyWarning
{
    [CmdletBinding()]
    Param
    (
    )

    Process
    {
        Write-Warning "This is a warning."
        Write-Host "This is after the warning."
    }
}

try
{
    Test-MyWarning -WarningAction Stop
}
catch
{
    Write-Host "Exception Caught!"
}

Output of that script:

WARNUNG: This is a warning.
Exception Caught!

[Splaxi]

@valerymoskalenko I appreciate you being honest, and stating that you don't agree with the decision. So thanks for that! On a personal note, everybody that skips in with work and hours, gets a saying in where the module should move, including the direction on how we should handle errors vs. warnings.

On to the issue at hand, I just tested @devax and his suggestion. He is totally rigt, the result is the an exception like you want things to work. Thing is, I did not implement Write-Warning - so right now, things will not work for you, as expected.

But this is a learning experience for all of us. I propose 2 possible solutions, that requires some work, before it is implemented across the entire module.

1. Refactor all Write-PSFMessage -Level Host to Write-PSFMessage -Level Warning
   This will, together with @devax and his approach, make it possible for you to break execution, if you simply add the -WarningAction Stop
   This is fairly easy to implement, it will be more consist of what we actually are trying to achieve, warn the user, and it makes it possible for both "camps" to have things work like they desire. But putting a bit more work onto you, because you need to remember the -WarningAction Stop for all commands you believe should fail with exceptions.

2. Refactor all Stop-PSFFunction -Message to support -EnableException
   This should also be fairly easy to implement, defaulting to $False. This will keep things working as they are now, but enable you to have things to break, without adding extra parameters to all your calls. I'll create a configuration that handles the default approach, on a user level. This either comes with a new cmdlet or simply documentation on how to change it using the PSFConfig* cmdlets that we rely on.

Suggestion 2 actually seems to be the best fit, because you centrally and only once, decide how you want things to work for you.

On the color of things, I need to document how you change the different colors using the PSFConfig* cmdlets that we rely on, then you can change the colors to match your desires. You should be able to change every color, for each type of message and parts of the message.

Let's continue the discussion, before I start refactoring things. I could totally use some help with the refactor. I can create and document the first cmdlet / function, and then you guys could help out making the changes.

Let me know what you think, both of you! @valerymoskalenko @devax

[devax]

@Splaxi Thank you for looking into this! I try to add something to the discussion.

IMHO, suggestion 1 should be implemented, of course only there, where you actually want to output a warning. This seems to be independent of the issue described in this topic.

I have not realized that you are using Stop-PSFunction, so I had an other look at that. I can confirm, Stop-PSFunctions seems not to respect the -WarningAction value of the current function it is running in.
Nevertheless, it does respect the global $WarningPreference variable, so one could use that variable to caught the warning as an exception. I have added a modified POC script below to demonstrate that.

The core question of the discussion seems to be: Is a warning appropriate in this situation or would an Error be more adequate? Like @valerymoskalenko has already stated, this is debatable and everyone might interpret it differently. I tend to agree with @valerymoskalenko: when I call a function named Switch-D365ActiveDatabase I expect it to do its task (switch the database) or fail, if it is unable to do it. Having that said, I think this is not so important if it actually gives you an error or a warning on failure. Important is, that there is a way to handle all different situations. With the so far presented options (the ones from your first reply @Splaxi and/or the usage of $WarningPreference), there should be enough room to use that function in its current state. Suggestion 2 might be a nice to have, but IMHO not really necessary.

# Set your warning preference globally: Will handle warnings like exceptions
$WarningPreference="Stop"

function Test-MyWarning2
{
    [CmdletBinding()]
    Param
    (
    )

    Process
    {
        Stop-PSFFunction -Message "I behave like a warning, but I do not respect WarningAction parameter handed over to me."
        Write-Host "After Stop-PSFunction call"
    }
}

try
{
    Test-MyWarning2
}
catch
{
    Write-Host "Exception Caught!"
}

Output:

WARNUNG: [16:32:49][Test-MyWarning2] I behave like a warning, but I do not respect WarningAction parameter handed over to me.
Exception Caught!

[Splaxi]

@devax Thank you for taking the time to share your ideas and help me understand things better. Your examples are a great help in the discussion!

Let me start with explaining the idea behind the module, it may not bring anything new to the table, but I feel it is important in this discussion, because it is so fundamental.

The core idea is to help people complete their "daily" tasks easier and more efficient, in a structured and repeatable manner. I want people to be able to produce the same output or results, regardless of their skill level and make them capable of predicting how long a given task will take.

I choose PowerShell as the platform for several reasons.

1. I wanted to become more proficient in PowerShell, which is rather selfish but true.
2. I saw a lack of adoption of PowerShell as a tool from other community members.
3. Microsoft didn't give us enough (intelligent) tools to help us while working with D365FO.
4. Providing building blocks, so you can build your own "work flow" that fits your needs.

The idea is that the tools help and guide people when they start using it, making sure that they don't get a "sea of red", PowerShell community lingo for exceptions, that scares them off. The exceptions don't provide any guidance or explaining what went wrong. It actually takes a lot of effort analyze and understand a PowerShell exception, especially for people that haven't been using PowerShell much - heck, I still struggle with understanding PowerShell exceptions.

So - this is why the current implementation is how it is. The module is split in being a helping hand for a person in his daily life, and along way help automate and produce repeatable results. The automation part is an added benefit, that comes from using PowerShell.

Back to the discussion!

Thanks to you @devax, I learned about -WarningAction Stop and $WarningPreference="Stop" these days. It actually shows that I have a lot to learn about the basic PowerShell stuff, even though I'm building a module.

I'm very depended on the PSFramework module, which I don't understand fully, but seek advice from the mastermind behind it all the time. Which brings me back to a simple solution that he shared with me

$PSDefaultParameterValues['*:EnableException'] = $true

This should ensure that Stop-PSFunction will throw an exception, like a real exception, whenever it is fired. With the current implementation his approach doesn't give the same error message details, as the $WarningPreference="Stop" but that is because we need to add -Exception ([System.Exception]::new("TEXT")) where we fill in the same text value that the Write-PSFMessage does. This is a small change, that shouldn't be to hard to implement.

So, to sum up what solutions we have, to enable you guys get the desired exception handling is:

1. $WarningPreference="Stop" - Which is the most PowerShell natural and widely documented approach.
2. $PSDefaultParameterValues['*:EnableException'] = $true - Which is very specific for PSFramework, which this module is heavily depended on.

These things don't require any code change, and will make sure that we all get a tool that can help us in our daily life.

@valerymoskalenko Please join the discussion when you get time on your hands, so we can make sure we cover all the different aspects.

Finally - thank you both for caring enough about the module, to give some feedback. Without feedback, I'm just building things the way I like them to be 😉

Updated: I just "documented" how to get the desired exception flow on the wiki, so we have a starting point. https://github.com/d365collaborative/d365fo.tools/wiki/Exception-handling

[devax]

Thank you @Splaxi for laying out the core ideas behind the module. Your 4 reasons for choosing Power Shell seem very valid to me and I think PS is the most fitting platform for the task. Also, you have created something very useful that I'm using nearly daily, so thank you for that.

I agree with everything you said, except for this part:

The idea is that the tools help and guide people when they start using it, making sure that they don't get a "sea of red", PowerShell community lingo for exceptions, that scares them off. The exceptions don't provide any guidance or explaining what went wrong. It actually takes a lot of effort analyze and understand a PowerShell exception, especially for people that haven't been using PowerShell much - heck, I still struggle with understanding PowerShell exceptions.

Going into details:

The idea is that the tools help and guide people when they start using it, making sure that they don't get a "sea of red", PowerShell community lingo for exceptions, that scares them off.

I don't think turning errors into warnings is a viable strategy to approach this problem. What people want is that "things work" when they use a command. This means to do everything you can to prevent that things go wrong. Example: I see you using a lot of default values for parameters. This is great and I consider this to be an adequate approach for this demand. Changing errors/exceptions into warnings on the other hand is not an adequate approach to this demand IMHO. When things go wrong, a user should be informed that things went wrong and the appropriate way to inform the user is to use errors/exceptions (at least in PS). Using warnings instead of an error does not make things "work better".
The goal should be: Try to prevent errors to happen. But if they do happen, don't hide them in warnings.

The exceptions don't provide any guidance or explaining what went wrong.

I must disagree here. A lot of effort went into the creation of meaningful exception messages that are very useful to the user. Personally, I consider Power Shell to be one of the most exemplary environments when it comes to error messages. Nearly daily I come across well written error messages in Power Shell, I could easily back this up with examples, if you want me to do so. Of course there are always the exceptions. But it is a new idea to me that people consider Power Shell error messages to be not easy to understand in general. In addition: Turning an error message into a warning message does not make it better readable or understandable, IMHO. Also, technically, every message presented as a warning message could also be presented as an error message.

The concept of terminating and non terminating errors might be new to new Power Shell users and also the tools you have at hand like the ErrorAction parameter for commandlets and the $ErrorActionPreference variable need some initial effort to familiarise yourself with. Once mastered, it gives you a lot of control over your tools. I think any Power Shell module should try to leverage the design principles behind these tools instead of trying to make up their own, differing design principles. But that's just my opinion, everyone can have his/her own. ;)

Coming back to the problem at hand:

• Applying the principle "things should just work" could mean that the scrip automatically removes the database if it already exists. As this might not be what the user wanted, the command could prompt the user if he wants to proceed (not uncommon with Power Shell). In addition, a -Force parameter could be added to remove it without prompting, so that it can be used in automation.
• In case of an error (switch process could not complete), an error should be presented to the user instead of a warning. This allows more natural error handling in automation scripts.

Sorry this became such a long post.

[FriedrichWeinmann]

Hi Folks,

jumping in on this discussion :)
@devax : Unfortunately, as long experience with PowerShell beginners shows: Throwing lots of red stuff does not help them troubleshoot the issue, it causes them to sadly turn to the next google solution.

With that, I try to govern the default behavior I use depending on the target audience: If it is something likely to be consumed by beginners, then it writes warnings by default and rarely writes errors.
If it is something targeted at advanced users, the other way around (if I bother with warnings at all, generally not so).

The important thing to keep in mind is that if you default to warnings, you need to have a way to turn on errors instead so try/catch and similar tools can be used. I suppose I should find a way to offer better docs on that and make it more intuitive to implement (One of the core reasons for Stop-PSFFunction to exist is to make implementing that simple).

To make opting into exceptions possible however, you need to implement that toggle and pass it through. The basic idea is ...

1. Add a toggle parameter to the implementing command:

[switch]
$EnableException

And then either ...

2a. Explicitly pass through the parameter

Stop-PSFFunction -Message "Foo" -EnableException $EnableException

or

2b. Implicitly pass through the parameter

which requires your module to opt into implicit inheritance by running the following line once during import:

Set-PSFFeature -Name PSFramework.InheritEnableException -Value $true -ModuleName 'd365fo.tools'

(Note: For implicit inheritance to work, your toggle parameter must be named $EnableException)

Now, you can agree or disagree with this design decision, I'll stick with the argument in favor though.
Seasoned users can always just update their profile to permanently switch it by adding a default parameter value. Making expectations at beginners on the other hand just doesn't work.

Btw, all messages - including error records that were attached - can be read after the fact by using Get-PSFMessage.

Cheers,
Fred

[Splaxi]

Hey,

The issue is not dead and I'm going to implement a helper function for you guys to activate as the first step of your scripts, to mimic the desired behavior.

I'm implementing the helper function in the integrations module first and will see how it behaves and take the learnings from that into this module.

Stay tuned!

[Splaxi]

And I finally had the time to implement the needed helper function.

0.5.40 is the release that you guys need to install!

Running Enable-D365Exception as the VERY first cmdlet in any script / PowerShell console will enable exceptions.

Not all cmdlets will throw exceptions from the beginning. Let me know when you hit one that should throw exceptions. Better yet, look at those that does it and help me implement the feature in the ones that you believe should be exception enabled.

[valerymoskalenko]

@Splaxi
Could you add it to New-D365Bacpac -ExportModeTier2 -SqlUser "sqladmin" -SqlPwd '***********' -NewDatabaseName $NewDBName -BacpacFile "D:\temp$NewDBName.bacpac" -Verbose

VERBOSE: [08:42:10][Test-TrustedConnection] Not capable of using Trusted Connection based on Tier validation.
VERBOSE: [08:42:10][Test-PathExists] Testing the path: D:\temp
VERBOSE: [08:42:10][New-D365Bacpac] Invoking the Tier 2 - Creation of Azure DB copy
VERBOSE: [08:42:10][Get-SQLCommand] Building the SQL connection string.
VERBOSE: [08:42:10][Invoke-AzureBackupRestore] Starting the cloning process of the Azure DB.
VERBOSE: [08:43:01][Get-SQLCommand] Building the SQL connection string.
VERBOSE: [08:43:01][Invoke-AzureBackupRestore] Start to wait for the cloning process of the Azure DB to complete.
VERBOSE: [08:43:01][Invoke-AzureBackupRestore] Cloning not complete Sleeping for 60 seconds. [08:43:01]
VERBOSE: [08:44:01][Invoke-AzureBackupRestore] Cloning not complete Sleeping for 60 seconds. [08:44:01]
VERBOSE: [08:45:01][Invoke-AzureBackupRestore] Cloning not complete Sleeping for 60 seconds. [08:45:01]
VERBOSE: [08:46:01][Invoke-AzureBackupRestore] Total time spent inside the function was 00:03:51.0815334
VERBOSE: [08:46:01][New-D365Bacpac] Invoking the Tier 2 - Clear Azure DB objects
Get-SQLCommand : A parameter cannot be found that matches parameter name 'EnableException'.
At line:2327 char:34
+     $sqlCommand = Get-SQLCommand @PsBoundParameters -TrustedConnectio ...
+                                  ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Get-SQLCommand], ParameterBindingException
    + FullyQualifiedErrorId : NamedParameterNotFound,Get-SQLCommand
 
The property 'CommandText' cannot be found on this object. Verify that the property exists and can be set.
At line:2333 char:5
+     $sqlCommand.CommandText = $commandText
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (:) [], RuntimeException
    + FullyQualifiedErrorId : PropertyNotFound
 
Write-PSFMessage : You cannot call a method on a null-valued expression.
At line:2346 char:9
+         Write-PSFMessage -Level Host -Message $messageString -Excepti ...
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:String) [Write-PSFMessage], RuntimeException
    + FullyQualifiedErrorId : d365fo.tools_Invoke-ClearAzureSpecificObjects,PSFramework.Commands.WritePSFMessageCommand

[valerymoskalenko]

Heh.. How to disable it now...

UPDATE: Disable is $PSDefaultParameterValues['*:EnableException'] = $false
But it not fix the error...

[Splaxi]

Thx for the feedback.

If you start a new powershell session / console, or simply import-module d365fo.tools -force then things should get back to "normal".

I did a quick fix and push, give it 5-10 minutes and then make sure to update to version 0.5.41

[Splaxi]

And it is released...