Make terminating errors terminate the right way

1-Draft/RFCNNNN-Make-Terminating-Errors-Terminate-The-Right-Way.md

@@ -0,0 +1,169 @@
+---
+RFC: RFCnnnn
+Author: Kirk Munro
+Status: Draft
+SupercededBy: 
+Version: 1.0
+Area: Engine
+Comments Due: July 15, 2019
+Plan to implement: Yes
+---
+
+# Make terminating errors terminate the right way in PowerShell
+
+By default in PowerShell, terminating errors do not actually terminate. For example, if you invoke this command in global scope, you will see the output "Why?" after the terminating error caused by the previous command:
+
+```PowerShell
+& {
+    1/0
+    'Why?'
+}
+```
+
+PowerShell has upheld this behaviour since version 1.0 of the language. You can make the terminating error actually terminate execution of the command, by wrapping the command in try/catch, like this:
+
+```PowerShell
+try {
+    1/0
+    'Why?'
+} catch {
+    throw
+}
+```
+
+In that example, the exception raised by dividing by zero properly terminates execution of the running command.
+
+The difference between these two examples poses a risk to scripters who share scripts or modules with the community. The risk is that end users using a shared resource such as a script or module may see different behaviour from the logic within that module depending on whether or not they were inside of a `try` block when they invoked the script or a command exported by the module. That risk is very undesirable, and as a result many community members who share scripts/modules with the community wrap their logic in a `try/catch{throw}` (or similar) scaffolding to ensure that the behavior of their code is consistent no matter where or how it was invoked.
+
+Now consider this code snippet:
+
+```PowerShell
+New-Module -Name ThisShouldNotImport {
+    $myList = [System.Collections.Generics.List[string]]::new()
+
+    function Test-RFC {
+        [CmdletBinding()]
+        param()
+        'Some logic'
+        1 / 0 # Oops
+        'Some more logic'
+    }
+} | Import-Module
+```
+
+If you invoke that snippet, the `ThisShouldNotImport` module imports successfully because the terminating error (`[System.Collections.Generics.List[string]]` is not a valid type name) does not actually terminate the loading of the module. This could cause your module to load in an unexpected state, which is a bad idea. If you loaded your module by invoking a command defined with that module, you won't see the terminating error that was raised during the loading of the module (the terminating error that was raised during the loading of the module is not shown at all in that scenario!), so you could end up with some undesirable behaviour when that command executes even though the loading of the module generated a "terminating" error, and not have a clue why. Further, the Test-RFC command exported by this module produces a terminating error, yet continues to execute after that error. Last, if the caller either loads your module or invokes your command inside of a `try` block, they will see different behaviour. Any execution of code beyond a terminating error should be intentional, not accidental like it is in both of these cases, and it most certainly should not be influenced by whether or not the caller loaded the module or invoked the command inside of a `try` block. Binary modules do not behave this way. Why should script modules be any different?
+
+Now have a look at the same module definition, this time with some extra scaffolding in place to make sure that terminating errors actually terminate:
+
+```PowerShell
+New-Module -Name ThisShouldNotImport {
+    trap {
+        break
+    }
+    $myList = [System.Collections.Generic.List[string]]::new()
+
+    function Test-RFC {
+        [CmdletBinding()]
+        param()
+        $callerEAP = $ErrorActionPreference
+        try {
+            'Some logic'
+            1 / 0 # Oops
+            'Some more logic'
+        } catch {
+            Write-Error -ErrorRecord $_ -ErrorAction $callerEAP
+        }
+    }
+} | Import-Module
+```
+
+With this definition, if the script module generates a terminating error, the module will properly fail to load (note, however, that the type name has been corrected in case you want to try this out). Further, if the command encounters a terminating error, it will properly terminate execution and the error returned to the caller will properly indicate that the `Test-RFC` command encountered an error. This scaffolding is so helpful that members of the community apply it to every module and every function they define within that module, just to get things to work the right way in PowerShell.
+
+All of this is simply absurd. Any script module that generates a terminating error in the module body should fail to import without extra effort, with an appropriate error message indicating why it did not import. Any advanced function defined within a script module that encounters a terminating error should terminate gracefully, such that the error message indicates which function the error came from, without requiring extra scaffolding code to make it work that way.
+
+Between the issues identified above, and the workarounds that include anti-patterns (naked `try/catch` blocks and `trap{break}` statements are anti-patterns), the PowerShell community is clearly in need of a solution that automatically resolves these issues in a non-breaking way.
+
+## Motivation
+
+As a script, function, or module author,
+I can write scripts with confidence knowing that terminating errors will terminate those commands the right way, without needing to add any scaffolding to correct inappropriate behaviours in PowerShell
+so that I can keep my logic focused on the work that needs to be done.
+
+## User experience
+
+The way forward for this issue is to add an optional feature (see: RFCNNNN-OptionalFeatures) that makes terminating errors terminate correctly. The script below demonstrates that a manifest can be generated with the `ImplicitTerminatingErrorHandling` optional feature enabled, and with that enabled the module author can write the script module and the advanced functions in that module knowing that terminating errors will be handled properly. No scaffolding is required once the optional feature is enabled, because it will correct the issues that need correcting to make this just work the right way, transparently.
+
+```powershell
+$moduleName = 'ModuleWithBetterErrorHandling'
+$modulePath = Join-Path -Path $([Environment]::GetFolderPath('MyDocuments')) -ChildPath PowerShell/Modules/${moduleName}
+New-Item -Path $modulePath -ItemType Directory -Force > $null
+$nmmParameters = @{
+    Path = "${modulePath}/${moduleName}.psd1"
+    RootModule = "./${moduleName}.psm1"
+    FunctionsToExport = @('Test-ErrorHandling')
+}
+
+#
+# Create the module manifest, enabling the optional ImplicitTerminatingErrorHandling feature in the module it loads
+#
+New-ModuleManifest @nmmParameters -OptionalFeatures ImplicitTerminatingErrorHandling
+
+$scriptModulePath = Join-Path -Path $modulePath -ChildPath "${moduleName}.psm1"
+New-Item -Path $scriptModulePath -ItemType File | Set-Content -Encoding UTF8 -Value @'
+    # If the next command is uncommented, Import-Module would fail when trying to load
+    # this module due to the terminating error actually terminating like it should
+    # 1/0 # Oops!
+    function Test-ErrorHandling {
+        [cmdletBinding()]
+        param()
+        'Some logic'
+        # The next command generates a terminating error, which will be treated as
+        # terminating and Test-ErrorHandling will fail properly, with the error text
+        # showing details about the Test-ErrorHandling invocation rather than details
+        # about the internals of Test-ErrorHandling.
+        Get-Process -Id 12345678 -ErrorAction Stop
+        'Some more logic'
+    }
+'@
+```
+
+Module authors who want this behaviour in every module they create can invoke the following command to make it default for module manifests created with `New-ModuleManifest`.
+
+```PowerShell
+Enable-OptionalFeature -Name ImplicitTerminatingErrorHandling -NewModuleManifests
+```
+
+Scripters wanting the behaviour in their scripts can use the #requires statement:
+
+```PowerShell
+#requires -OptionalFeatures ImplicitTerminatingErrorHandling
+```
+
+## Specification
+
+Implementation of this RFC would require the following:
+
+### Implementation of optional feature support
+
+See RFCNNNN-Optional-Features for more information.
+
+### Addition of the `ImplicitTerminatingErrorHandling` optional feature definition
+
+This would require adding the feature name and description in the appropriate locations so that the feature can be discovered and enabled.
+
+### PowerShell engine updates
+
+The PowerShell engine would have to be updated such that:
+
+* scripts invoked with the optional feature enabled treat terminating errors as terminating
+* scripts and functions with `CmdletBinding` attributes when this optional feature is enabled treat terminating errors as terminating and gracefully report errors back to the caller (i.e. these commands should not throw exceptions)
+
+## Alternate proposals and considerations
+
+### Make this optional feature on by default for new module manifests
+
+This feature is so useful that I would recommend it as a best practice. If making it just work this way globally wouldn't incur a breaking change in PowerShell, I would want it to always work that way by default. Since making it work this way globally would incur a breaking change, my recommendation is to make this optional feature on in new module manifests by default so that anyone not wanting it to work this way has to turn the optional feature off. That corrects the behaviour going forward while allowing authors of older modules/scripts can opt-in to the feature when they are ready.
+
+### Related issue
+
+[PowerShell Issue #9855](https://github.com/PowerShell/PowerShell/issues/9855) is very closely related to this RFC, and it would be worth considering fixing that issue as part of this RFC if it is not already resolved at that time.

1-Draft/RFCNNNN-Optional-Features.md

@@ -0,0 +1,165 @@
+---
+RFC: RFCnnnn
+Author: Kirk Munro
+Status: Draft
+SupercededBy: 
+Version: 1.0
+Area: Engine
+Comments Due: July 15, 2019
+Plan to implement: Yes
+---
+
+# Optional features in PowerShell
+
+There are several important issues in the PowerShell language that cannot be corrected without introducing breaking changes. At the same time, the number of breaking changes introduced in a new version of PowerShell needs to be as minimal as possible, so that there is a low barrier to adoption of new versions, allowing community members can transition scripts and modules across versions more easily. Given that those two statements are in conflict with one another, we need to consider how we can optionally introduce breaking changes into PowerShell over time.
+
+PowerShell has support for experimental features, which some may think covers this need; however, the intent of experimental features is to allow the community to try pre-release versions of PowerShell with breaking changes that are deemed necessary so that they can more accurately assess the impact of those breaking changes. For release versions of PowerShell, an experimental feature has one of three possible outcomes:
+
+1. The breaking change in the experimental feature is deemed necessary and accepted by the community as not harmful to adoption of new versions, in which case the experimental feature is no longer marked as experimental.
+1. The breaking change in the experimental feature is deemed necessary, but considered harmful to adoption of new versions, in which case the experimental feature is changed to an optional feature.
+1. The breaking change in the experimental feature is deemed not useful enough, in which case the experimental feature is deprecated.
+
+In some cases a breaking change may be implemented immediately as an optional feature, when it is known up front that such a breaking change would be considered harmful to adoption of new versions of PowerShell.
+
+Given all of that, we need to add support for optional features in PowerShell so that what is described above becomes a reality.
+
+As an example of a feature that will be optional if implemented, see RFCNNNN-Propagate-Execution-Preferences-Beyond-Module-Scope or RFCNNNN-Make-Terminating-Errors-Terminate.
+
+## Motivation
+
+As a script, function, or module author,
+I can enable optional features in my scripts or modules,
+so that I can leverage new functionality that could break existing scripts.
+
+## User experience
+
+```powershell
+# Create a module manifest, specifically enabling one or more optional features in the manifest
+New-ModuleManifest -Path ./test.psd1 -OptionalFeatures @('OptionalFeature1','OptionalFeature2') -PassThru | Get-Content
+
+# Output:
+#
+# @{
+#
+# <snip>
+#
+# # Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell.
+# PrivateData = @{
+#
+#     <snip>
+#
+#     PSData = @{
+#
+#         # Optional features enabled in this module.
+#         OptionalFeatures = @(
+#             'OptionalFeature1'
+#             'OptionalFeature2'
+#         )
+#
+#         <snip>
+#
+#     } # End of PSData hashtable
+#
+#     <snip>
+#
+# } # End of PrivateData hashtable
+#
+# }
+
+# Create a script file, enabling one or more optional features in the file
+@'
+#requires -OptionalFeature OptionalFeature1,OptionalFeature2
+
+<snip>
+'@ | Out-File -FilePath ./test.ps1
+
+# Get a list of optional features that are available
+Get-OptionalFeature
+
+# Output:
+#
+# Name                              EnabledIn Source                              Description
+# ----                                ------- ------                              -----------
+# OptionalFeature1                   Manifest PSEngine                            Description of optional feature 1
+# OptionalFeature2                    Session PSEngine                            Description of optional feature 2
+
+# Enable an optional feature by default in PowerShell
+Enable-OptionalFeature -Name OptionalFeature1
+
+# Output:
+# This works just like Enable-ExperimentalFeature, turning the optional
+# feature on by default for all future sessions in PowerShell.
+
+# Disable an optional feature by default in PowerShell
+Disable-OptionalFeature -Name OptionalFeature1
+
+# Output:
+# This works ust like Disable-ExperimentalFeature, turning the optional
+# feature off by default for all future sessions in PowerShell.
+
+# Enable an optional feature by default in all new module manifests
+# created with New-ModuleManifest in all future sessions in PowerShell.
+Enable-OptionalFeature -Name OptionalFeature1 -NewModuleManifests
+
+# Disable an optional feature by default in all new module manifests
+# created with New-ModuleManifest in all future sessions in PowerShell.
+Disable-OptionalFeature -Name OptionalFeature1 -NewModuleManifests
+```
+
+## Specification
+
+Aside from closely (but not exactly, see below) mirroring what is already in place internally for experimental features in PowerShell, this RFC includes a few additional enhancements that will be useful for optional features, as follows:
+
+### Add parameter to New-ModuleManifest
+
+`[-OptionalFeatures <string[]>]`
+
+This new parameter would assign specific optional features to new modules. Note that these would be in addition to optional features that are enabled by default in manifests created with `New-ModuleManifest`.
+
+### Add parameter to #requires statement
+
+`#requires -OptionalFeatures <string[]>`
+
+This new parameter would enable optional features in the current script file.
+
+### New command: Get-OptionalFeature
+
+```none
+Get-OptionalFeature [[-Name] <string[]>] [<CommonParameters>]
+```
+
+This command would return the optional features that are available in PowerShell. The default output format would be of type table with the properties `Name`, `Enabled`, `Source`, and `Description`. All of those properties would be of type string except for `Enabled`, which would be an enumeration with the possible values of `NotEnabled`, `Session`, `Manifest`, and `Script`. This differs from experimental features where `Enabled` is a boolean value. Given the locations in which an optional feature can be enabled, it would be more informative to identify where it is enabled than simply showing `$true` or `$false`.
+
+### New command: Enable-OptionalFeature
+
+```none
+Enable-OptionalFeature [-Name] <string[]> [-NewModuleManifests] [-WhatIf] [-Confirm] [<CommonParameters>]
+```
+
+This command would enable an optional feature either globally (if the `-NewModuleManifests` switch is not used) or only in new module manifests created by `New-ModuleManifest`.
+
+### New command: Disable-OptionalFeature
+
+```none
+Disable-OptionalFeature [-Name] <string[]> [-NewModuleManifests] [-WhatIf] [-Confirm] [<CommonParameters>]
+```
+
+This command would disable an optional feature either globally (if the `-NewModuleManifests` switch is not used) or only in new module manifests created by `New-ModuleManifest`. If the optional feature is not enabled that way in the first place, nothing would happen.
+
+### New command: Use-OptionalFeature
+
+```none
+Use-OptionalFeature [-Name] <string[]> [-ScriptBlock] <ScriptBlock> [-Confirm] [<CommonParameters>]
+```
+
+This command would enable an optional feature for the duration of the `ScriptBlock` identified in the `-ScriptBlock` parameter, and return the feature to its previous state afterwards. This allows for easy use of an optional feature over a small section of code.
+
+## Alternate proposals and considerations
+
+### Extend experimental features to support the enhancements defined in this RFC
+
+Experimental features and optional features are very similar to one another, so much so that they really only differ in name. Given the model for how both of these types of features are used, it may make sense to have them both use the same functionality when it comes to enabling/disabling them in scripts and modules. The downside I see to this approach is that optional features are permanent features in PowerShell while experimental features are not, so it may not be a good idea to support more permanent ways to enable experimental features such as `#requires` or enabling an experimental feature in a new module manifest.
+
+### Supporting a `-Scope` parameter like the experimental feature cmdlets do
+
+The `Enable-OptionalFeature` and `Disable-OptionalFeature` cmdlets could support a `-Scope` parameter like their experimental feature cmdlet counterparts do. I felt it was better to remove this for optional features, because it may be risky to allow a command to enable an optional feature in a scope above the one in which it is invoked, influencing behaviour elsewhere.