From 04f4f591b58da4c7921b0157243f3bdf3834bcbf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 14:04:38 +0000 Subject: [PATCH 01/15] Initial plan From bc95ebd2c6dbcf91e1aae14f19117eaf56c32bfb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 14:10:55 +0000 Subject: [PATCH 02/15] Update error handling documentation in CONTRIBUTING.md Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CHANGELOG.md | 3 + CONTRIBUTING.md | 206 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 150 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37af2ea9be..44643d9215 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Set-SqlDscDatabaseProperty` - Updated comment-based help to reference correct enum values. - Added SQL Server version requirements to version-specific parameter help. +- Updated CONTRIBUTING.md error handling documentation to clarify proper usage + of `Write-Error` vs `$PSCmdlet.ThrowTerminatingError()` in public commands + ([issue #2193](https://github.com/dsccommunity/SqlServerDsc/issues/2193)). ### Fixed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 47010ac410..b94b88aeb3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -308,85 +308,173 @@ If neither of those commands works in the scenarion then `throw` shall be used. Commands are publicly exported commands from the module, and the source for commands are located in the folder `./source/Public`. -#### Non-Terminating Error +#### Error Handling Guidelines -A non-terminating error should only be used when a command shall be able to -handle (ignoring) an error and continue processing and still give the user -an expected outcome. +Public commands should primarily use `Write-Error` for error handling, as it +provides the most flexible and predictable behavior for callers. The statement +`throw` shall never be used in public commands. -With a non-terminating error the user is able to decide whether the command -should throw or continue processing on error. The user can pass the -parameter and value `-ErrorAction 'SilentlyContinue'` to the command to -ignore the error and allowing the command to continue, for example the -command could then return `$null`. But if the user passes the parameter -and value `-ErrorAction 'Stop'` the same error will throw a terminating -error telling the user the expected outcome could not be achieved. +##### When to Use Write-Error -The below example checks to see if a database exist, if it doesn't a -non-terminating error are called. The user is able to either ignore the -error or have it throw depending on what value the user specifies -in parameter `ErrorAction` (or `$ErrorActionPreference`). +Use `Write-Error` in most scenarios where a public command encounters an error: + +- **Validation failures**: When input parameters fail validation +- **Resource not found**: When a requested resource doesn't exist +- **Operation failures**: When an operation cannot be completed +- **Permission issues**: When access is denied + +`Write-Error` generates a non-terminating error by default, allowing the caller +to control behavior via the `-ErrorAction` parameter. This is the expected +PowerShell pattern. + +**Example**: A command that retrieves a database returns an error if not found: ```powershell if (-not $databaseExist) { $errorMessage = $script:localizedData.MissingDatabase -f $DatabaseName - Write-Error -Message $errorMessage -Category 'InvalidOperation' -ErrorId 'GS0001' -TargetObject $DatabaseName + Write-Error -Message $errorMessage -Category 'ObjectNotFound' -ErrorId 'GSD0001' -TargetObject $DatabaseName + + return } ``` -#### Terminating Error +The caller controls the behavior: -A terminating error is an error that the user are not able to ignore by -passing a parameter to the command (like for non-terminating errors). +```powershell +# Returns $null, continues execution (non-terminating) +$db = Get-Database -Name 'NonExistent' -If a command shall throw an terminating error then the statement `throw` shall -not be used, neither shall the command `Write-Error` with the parameter -`-ErrorAction Stop`. Always use the method `$PSCmdlet.ThrowTerminatingError()` -to throw a terminating error. The exception is when a `[ValidateScript()]` -has to throw an error, then `throw` must be used. +# Throws terminating error, stops execution +$db = Get-Database -Name 'NonExistent' -ErrorAction 'Stop' +``` > [!IMPORTANT] -> Below output assumes `$ErrorView` is set to `'NormalView'` in the -> PowerShell session. - -When using `throw` it will fail on the line with the throw statement -making it look like it is that statement inside the function that failed, -which is not correct since it is either a previous command or evaluation -that failed resulting in the line with the `throw` being called. This is -an example when using `throw`: - -```plaintext -Exception: -Line | - 2 | throw 'My error' - | ~~~~~~~~~~~~~~~~ - | My error -``` +> Always use `return` after `Write-Error` to prevent further processing in the +> command. Without `return`, the command continues executing, which may cause +> unexpected behavior or additional errors. + +##### Understanding $PSCmdlet.ThrowTerminatingError -When instead using `$PSCmdlet.ThrowTerminatingError()`: +The `$PSCmdlet.ThrowTerminatingError()` method is **not recommended** for public +commands despite its name suggesting it throws a terminating error. It actually +throws a **command-terminating error** which has unexpected behavior when called +from another command. + +**The Problem**: When a command uses `$PSCmdlet.ThrowTerminatingError()`, the +error behaves like a non-terminating error to the caller unless the caller +explicitly uses `-ErrorAction 'Stop'`. This creates confusing behavior: ```powershell -$PSCmdlet.ThrowTerminatingError( - [System.Management.Automation.ErrorRecord]::new( - 'MyError', - 'GS0001', - [System.Management.Automation.ErrorCategory]::InvalidOperation, - 'MyObjectOrValue' +function Get-Something +{ + [CmdletBinding()] + param () + + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + 'Database not found', + 'GS0001', + [System.Management.Automation.ErrorCategory]::ObjectNotFound, + 'MyDatabase' + ) ) -) + + # This line never executes (correctly stopped) + Write-Output 'After error' +} + +function Start-Operation +{ + [CmdletBinding()] + param () + + Get-Something + + # BUG: This line DOES execute even though Get-Something threw an error! + Write-Output 'Operation completed' +} + +# Caller must use -ErrorAction 'Stop' to prevent continued execution +Start-Operation -ErrorAction 'Stop' ``` -The result from `$PSCmdlet.ThrowTerminatingError()` shows that the command -failed (in this example `Get-Something`) and returns a clear category and -error code. - -```plaintext -Get-Something : My Error -At line:1 char:1 -+ Get-Something -+ ~~~~~~~~~~~~~ -+ CategoryInfo : InvalidOperation: (MyObjectOrValue:String) [Get-Something], Exception -+ FullyQualifiedErrorId : GS0001,Get-Something +Because of this behavior, **use `Write-Error` instead** in public commands. + +##### When $PSCmdlet.ThrowTerminatingError Can Be Used + +Use `$PSCmdlet.ThrowTerminatingError()` only in these limited scenarios: + +1. **Private functions** that are only called internally and where the behavior + is well understood +2. **Assert-style commands** (like `Assert-SqlDscLogin`) whose purpose is to + throw on failure +3. **Commands with `SupportsShouldProcess`** where an error in the + `ShouldProcess` block must terminate (before state changes) + +In these cases, ensure callers are aware of the behavior and use +`-ErrorAction 'Stop'` when calling the command from other commands. + +##### Exception Handling in Commands + +When catching exceptions in try-catch blocks, re-throw errors using `Write-Error` +with the original exception: + +```powershell +try +{ + $database.Create() +} +catch +{ + $errorMessage = $script:localizedData.CreateDatabaseFailed -f $DatabaseName + + Write-Error -Message $errorMessage -Category 'InvalidOperation' -ErrorId 'CD0001' -TargetObject $DatabaseName -Exception $_.Exception + + return +} ``` + +##### Pipeline Processing Considerations + +For commands that accept pipeline input and process multiple items, use +`Write-Error` to allow processing to continue for remaining items: + +```powershell +process +{ + foreach ($item in $Items) + { + if (-not (Test-ItemValid $item)) + { + Write-Error -Message "Invalid item: $item" -Category 'InvalidData' -ErrorId 'PI0001' -TargetObject $item + continue + } + + # Process valid items + Process-Item $item + } +} +``` + +The caller can control whether to stop on first error or continue: + +```powershell +# Continue processing all items, collect errors +$results = $items | Process-Items + +# Stop on first error +$results = $items | Process-Items -ErrorAction 'Stop' +``` + +##### Summary + +| Scenario | Use | Why | +|----------|-----|-----| +| General error handling in public commands | `Write-Error` | Provides consistent, predictable behavior; allows caller to control termination | +| Pipeline processing with multiple items | `Write-Error` | Allows processing to continue for remaining items | +| Assert-style commands | `$PSCmdlet.ThrowTerminatingError()` | Command purpose is to throw on failure | +| Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | +| Any command | Never use `throw` | Poor error messages; unpredictable behavior | +| Parameter validation attributes | `throw` | Only valid option within `[ValidateScript()]` | From 259bfbb615316f64a4350e2af7d730a6dddd73f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 14:12:14 +0000 Subject: [PATCH 03/15] Fix markdown line length for summary table Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b94b88aeb3..3c88d66cf8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -470,6 +470,7 @@ $results = $items | Process-Items -ErrorAction 'Stop' ##### Summary + | Scenario | Use | Why | |----------|-----|-----| | General error handling in public commands | `Write-Error` | Provides consistent, predictable behavior; allows caller to control termination | @@ -478,3 +479,4 @@ $results = $items | Process-Items -ErrorAction 'Stop' | Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | | Any command | Never use `throw` | Poor error messages; unpredictable behavior | | Parameter validation attributes | `throw` | Only valid option within `[ValidateScript()]` | + From 29df2fb95a697ee6d5a65f7bab7d2111f15ddac6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 14:14:41 +0000 Subject: [PATCH 04/15] Fix typos in Class-based DSC resource section Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3c88d66cf8..fa00a4258f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -297,11 +297,11 @@ be compiled to a .mof file. If the tests find any errors the build will fail. A terminating error is an error that prevents the resource to continue further. If a DSC resource shall throw an terminating error the commands of the module **DscResource.Common** shall be used primarily; [`New-ArgumentException`](https://github.com/dsccommunity/DscResource.Common#new-invalidargumentexception), -[`New-InvalidDataExcpetion`](https://github.com/dsccommunity/DscResource.Common#new-invaliddataexception), +[`New-InvalidDataException`](https://github.com/dsccommunity/DscResource.Common#new-invaliddataexception), [`New-InvalidOperationException`](https://github.com/dsccommunity/DscResource.Common#new-invalidoperationexception), [`New-InvalidResultException`](https://github.com/dsccommunity/DscResource.Common#new-invalidresultexception), or [`New-NotImplementedException`](https://github.com/dsccommunity/DscResource.Common#new-notimplementedexception). -If neither of those commands works in the scenarion then `throw` shall be used. +If neither of those commands works in the scenario then `throw` shall be used. ### Commands From e24f11c1df502500b31ce2280c1cedbe53f85d42 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 16:52:14 +0000 Subject: [PATCH 05/15] Document ValidateScript exception and clarify ErrorActionPreference usage Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 110 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa00a4258f..d7b7a6347a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -312,7 +312,8 @@ commands are located in the folder `./source/Public`. Public commands should primarily use `Write-Error` for error handling, as it provides the most flexible and predictable behavior for callers. The statement -`throw` shall never be used in public commands. +`throw` shall never be used in public commands except within parameter +validation attributes like `[ValidateScript()]` where it is the only option. ##### When to Use Write-Error @@ -416,6 +417,106 @@ Use `$PSCmdlet.ThrowTerminatingError()` only in these limited scenarios: In these cases, ensure callers are aware of the behavior and use `-ErrorAction 'Stop'` when calling the command from other commands. +##### Handling Errors from .NET Methods and Cmdlets + +When working with .NET methods (like SMO objects) and PowerShell cmdlets, understand +how exceptions are caught: + +**For .NET methods (.NET Framework/Core APIs and SMO):** +- Exceptions are **always caught** in try-catch blocks +- No need to set `$ErrorActionPreference` - the exceptions throw naturally +- Example: `$alertObject.Drop()`, `$database.Create()`, `[System.IO.File]::OpenRead()` + +**For PowerShell cmdlets:** +- Use `-ErrorAction 'Stop'` to make errors catchable in try-catch +- **Do not** set `$ErrorActionPreference = 'Stop'` when using `-ErrorAction 'Stop'` - it's redundant +- Only set `$ErrorActionPreference = 'Stop'` if you need blanket error handling for multiple cmdlets + +**Recommended pattern for .NET methods:** + +```powershell +try +{ + # .NET methods throw exceptions automatically - no ErrorActionPreference needed + $alertObject.Drop() + + Write-Verbose -Message ($script:localizedData.AlertRemoved -f $alertObject.Name) +} +catch +{ + $errorMessage = $script:localizedData.RemoveFailed -f $alertObject.Name + + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + [System.InvalidOperationException]::new($errorMessage, $_.Exception), + 'RSAA0001', + [System.Management.Automation.ErrorCategory]::InvalidOperation, + $alertObject + ) + ) +} +``` + +**Pattern for cmdlets with blanket error handling:** + +Only use `$ErrorActionPreference = 'Stop'` when you have multiple cmdlets +and don't want to add `-ErrorAction 'Stop'` to each one: + +```powershell +try +{ + $originalErrorActionPreference = $ErrorActionPreference + $ErrorActionPreference = 'Stop' + + # Multiple cmdlets without -ErrorAction on each + $instance = Get-SqlDscManagedComputerInstance -InstanceName $InstanceName + $protocol = Get-SqlDscServerProtocolName -ProtocolName $ProtocolName + $serverProtocol = $instance.ServerProtocols[$protocol.ShortName] +} +catch +{ + # Handle error +} +finally +{ + $ErrorActionPreference = $originalErrorActionPreference +} +``` + +> [!IMPORTANT] +> **Do not** use the pattern `$ErrorActionPreference = 'Stop'` followed by +> `-ErrorAction 'Stop'` on the same cmdlet - this is redundant. The +> `-ErrorAction` parameter always takes precedence over `$ErrorActionPreference`. + +> [!NOTE] +> The pattern of setting `$ErrorActionPreference = 'Stop'` in a try block +> with restoration in finally is useful for blanket error handling, but NOT +> necessary for .NET method calls (they always throw) or when using +> `-ErrorAction 'Stop'` on individual cmdlets. + +##### Parameter Validation with ValidateScript + +When using `[ValidateScript()]` attribute on parameters, you **must** use +`throw` for validation failures, as it is the only mechanism that works +within validation attributes: + +```powershell +[Parameter()] +[ValidateScript({ + if (-not (Test-Path -Path $_)) + { + throw ($script:localizedData.Audit_PathParameterValueInvalid -f $_) + } + + return $true + })] +[System.String] +$Path +``` + +This is the **only acceptable use** of `throw` in public commands and private +functions. + ##### Exception Handling in Commands When catching exceptions in try-catch blocks, re-throw errors using `Write-Error` @@ -475,8 +576,11 @@ $results = $items | Process-Items -ErrorAction 'Stop' |----------|-----|-----| | General error handling in public commands | `Write-Error` | Provides consistent, predictable behavior; allows caller to control termination | | Pipeline processing with multiple items | `Write-Error` | Allows processing to continue for remaining items | +| Catching .NET method exceptions | try-catch without setting `$ErrorActionPreference` | .NET exceptions are always caught automatically | +| Blanket error handling for multiple cmdlets | Set `$ErrorActionPreference = 'Stop'` in try, restore in finally | Avoids adding `-ErrorAction 'Stop'` to each cmdlet | +| Single cmdlet error handling | Use `-ErrorAction 'Stop'` on the cmdlet | Simpler and more explicit than setting `$ErrorActionPreference` | | Assert-style commands | `$PSCmdlet.ThrowTerminatingError()` | Command purpose is to throw on failure | | Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | -| Any command | Never use `throw` | Poor error messages; unpredictable behavior | -| Parameter validation attributes | `throw` | Only valid option within `[ValidateScript()]` | +| Parameter validation in `[ValidateScript()]` | `throw` | Only valid option within validation attributes | +| Any other scenario in commands | Never use `throw` | Poor error messages; unpredictable behavior | From 2d01ca3e2d016b0965c3756a20b4109c8c6d34f6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 16:54:11 +0000 Subject: [PATCH 06/15] Address code review comments and update AI instructions Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- ...-community-style-guidelines-powershell.instructions.md | 8 +++++--- CONTRIBUTING.md | 7 ++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md index c7a2e93536..effeb670e9 100644 --- a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md +++ b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md @@ -82,7 +82,7 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - `$PSCmdlet.ShouldProcess` must use required pattern - Inside `$PSCmdlet.ShouldProcess`-block, avoid using `Write-Verbose` - Never use backtick as line continuation in production code. -- Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'`; restore previous value directly after invocation (do not use try-catch-finally) +- For blanket error handling across multiple cmdlets, set `$ErrorActionPreference = 'Stop'` in try block and restore in finally block; for single cmdlet use `-ErrorAction 'Stop'` instead - Use `[Alias()]` attribute for function aliases, never `Set-Alias` or `New-Alias` ## Output streams @@ -92,11 +92,13 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - Use `Write-Verbose` for: High-level execution flow only; User-actionable information - Use `Write-Information` for: User-facing status updates; Important operational messages; Non-error state changes - Use `Write-Warning` for: Non-fatal issues requiring attention; Deprecated functionality usage; Configuration problems that don't block execution -- Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except for classes), use relevant error category, in try-catch include exception with localized message -- Use `Write-Error` for non-terminating errors +- Use `Write-Error` for most error scenarios in public commands - Always include `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), `-TargetObject` (object causing error) - In catch blocks, pass original exception using `-Exception` - Always use `return` after `Write-Error` to avoid further processing +- Use `$PSCmdlet.ThrowTerminatingError()` only in limited scenarios: assert-style commands, private functions, or catch blocks; note that callers must use `-ErrorAction 'Stop'` to catch these errors +- Use `throw` only in `[ValidateScript()]` parameter validation attributes +- .NET method exceptions (e.g., SMO methods) are always caught in try-catch blocks without needing to set `$ErrorActionPreference` ## ShouldProcess Required Pattern diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d7b7a6347a..fd36dbd779 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -434,6 +434,10 @@ how exceptions are caught: **Recommended pattern for .NET methods:** +This example shows using `$PSCmdlet.ThrowTerminatingError()` in a catch block, +which is acceptable when catching exceptions from .NET methods. The caller must +use `-ErrorAction 'Stop'` to catch this error. + ```powershell try { @@ -486,7 +490,8 @@ finally > [!IMPORTANT] > **Do not** use the pattern `$ErrorActionPreference = 'Stop'` followed by > `-ErrorAction 'Stop'` on the same cmdlet - this is redundant. The -> `-ErrorAction` parameter always takes precedence over `$ErrorActionPreference`. +> `-ErrorAction` parameter takes precedence over `$ErrorActionPreference` +> for that specific cmdlet call. > [!NOTE] > The pattern of setting `$ErrorActionPreference = 'Stop'` in a try block From 8eef0bf1d6024489d2e14672d719a73b5e764075 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 17:00:52 +0000 Subject: [PATCH 07/15] Correct ErrorActionPreference guidance based on testing Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 46 +++++++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fd36dbd779..ec5f2b410d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -313,7 +313,8 @@ commands are located in the folder `./source/Public`. Public commands should primarily use `Write-Error` for error handling, as it provides the most flexible and predictable behavior for callers. The statement `throw` shall never be used in public commands except within parameter -validation attributes like `[ValidateScript()]` where it is the only option. +validation attributes like `[ValidateScript()]` where it is the only valid +mechanism for validation failures. ##### When to Use Write-Error @@ -411,8 +412,12 @@ Use `$PSCmdlet.ThrowTerminatingError()` only in these limited scenarios: is well understood 2. **Assert-style commands** (like `Assert-SqlDscLogin`) whose purpose is to throw on failure -3. **Commands with `SupportsShouldProcess`** where an error in the - `ShouldProcess` block must terminate (before state changes) +3. **Commands with `SupportsShouldProcess`** where an operation failure in the + `ShouldProcess` block must terminate (common for state-changing operations + that cannot be partially completed) +4. **Catch blocks in state-changing commands** where a critical operation + failure must be communicated as a terminating error to prevent further + state changes In these cases, ensure callers are aware of the behavior and use `-ErrorAction 'Stop'` when calling the command from other commands. @@ -434,9 +439,13 @@ how exceptions are caught: **Recommended pattern for .NET methods:** -This example shows using `$PSCmdlet.ThrowTerminatingError()` in a catch block, -which is acceptable when catching exceptions from .NET methods. The caller must -use `-ErrorAction 'Stop'` to catch this error. +This example shows a state-changing command using `$PSCmdlet.ThrowTerminatingError()` +in a catch block. This is acceptable for commands with `SupportsShouldProcess` where +operation failures must terminate. Note that callers must use `-ErrorAction 'Stop'` +to catch this error. + +For non-state-changing commands (like Get/Test commands), prefer `Write-Error` in +catch blocks instead. ```powershell try @@ -461,10 +470,10 @@ catch } ``` -**Pattern for cmdlets with blanket error handling:** +**Pattern for blanket error handling:** -Only use `$ErrorActionPreference = 'Stop'` when you have multiple cmdlets -and don't want to add `-ErrorAction 'Stop'` to each one: +Use `$ErrorActionPreference = 'Stop'` when you need blanket error handling for +multiple cmdlets or when calling commands that use `$PSCmdlet.ThrowTerminatingError()`: ```powershell try @@ -487,6 +496,12 @@ finally } ``` +> [!NOTE] +> Setting `$ErrorActionPreference = 'Stop'` is particularly useful when calling +> commands that use `$PSCmdlet.ThrowTerminatingError()`. Without it (or +> `-ErrorAction 'Stop'` on each call), these command-terminating errors will not +> stop the calling function - it will continue executing after the error. + > [!IMPORTANT] > **Do not** use the pattern `$ErrorActionPreference = 'Stop'` followed by > `-ErrorAction 'Stop'` on the same cmdlet - this is redundant. The @@ -495,9 +510,12 @@ finally > [!NOTE] > The pattern of setting `$ErrorActionPreference = 'Stop'` in a try block -> with restoration in finally is useful for blanket error handling, but NOT -> necessary for .NET method calls (they always throw) or when using -> `-ErrorAction 'Stop'` on individual cmdlets. +> with restoration in finally is useful for: +> - Blanket error handling across multiple cmdlets +> - Catching errors from commands that use `$PSCmdlet.ThrowTerminatingError()` +> (without this, those errors won't stop the caller) +> - However, it's NOT necessary for .NET method calls (they always throw) or +> when using `-ErrorAction 'Stop'` on individual cmdlets. ##### Parameter Validation with ValidateScript @@ -582,9 +600,11 @@ $results = $items | Process-Items -ErrorAction 'Stop' | General error handling in public commands | `Write-Error` | Provides consistent, predictable behavior; allows caller to control termination | | Pipeline processing with multiple items | `Write-Error` | Allows processing to continue for remaining items | | Catching .NET method exceptions | try-catch without setting `$ErrorActionPreference` | .NET exceptions are always caught automatically | -| Blanket error handling for multiple cmdlets | Set `$ErrorActionPreference = 'Stop'` in try, restore in finally | Avoids adding `-ErrorAction 'Stop'` to each cmdlet | +| Blanket error handling for multiple cmdlets | Set `$ErrorActionPreference = 'Stop'` in try, restore in finally | Avoids adding `-ErrorAction 'Stop'` to each cmdlet; also catches ThrowTerminatingError from child commands | | Single cmdlet error handling | Use `-ErrorAction 'Stop'` on the cmdlet | Simpler and more explicit than setting `$ErrorActionPreference` | +| Calling commands that use ThrowTerminatingError | Set `$ErrorActionPreference = 'Stop'` or use `-ErrorAction 'Stop'` | Required to catch these command-terminating errors; without it, caller continues after error | | Assert-style commands | `$PSCmdlet.ThrowTerminatingError()` | Command purpose is to throw on failure | +| State-changing commands (catch blocks) | `$PSCmdlet.ThrowTerminatingError()` | Prevents partial state changes; caller must use `-ErrorAction 'Stop'` or set `$ErrorActionPreference` | | Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | | Parameter validation in `[ValidateScript()]` | `throw` | Only valid option within validation attributes | | Any other scenario in commands | Never use `throw` | Poor error messages; unpredictable behavior | From 78ecf3a40667c660f361dfc49166799435c0da0d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 17:37:45 +0000 Subject: [PATCH 08/15] Clarify that -ErrorAction Stop alone is insufficient with ThrowTerminatingError Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- ...tyle-guidelines-powershell.instructions.md | 10 +++++---- CONTRIBUTING.md | 22 ++++++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md index effeb670e9..bd35914ea5 100644 --- a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md +++ b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md @@ -82,7 +82,8 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - `$PSCmdlet.ShouldProcess` must use required pattern - Inside `$PSCmdlet.ShouldProcess`-block, avoid using `Write-Verbose` - Never use backtick as line continuation in production code. -- For blanket error handling across multiple cmdlets, set `$ErrorActionPreference = 'Stop'` in try block and restore in finally block; for single cmdlet use `-ErrorAction 'Stop'` instead +- For blanket error handling across multiple cmdlets, set `$ErrorActionPreference = 'Stop'` in try block and restore in finally block +- When calling commands that use `$PSCmdlet.ThrowTerminatingError()`, set `$ErrorActionPreference = 'Stop'` OR wrap in try-catch (using `-ErrorAction 'Stop'` alone is insufficient) - Use `[Alias()]` attribute for function aliases, never `Set-Alias` or `New-Alias` ## Output streams @@ -92,12 +93,13 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - Use `Write-Verbose` for: High-level execution flow only; User-actionable information - Use `Write-Information` for: User-facing status updates; Important operational messages; Non-error state changes - Use `Write-Warning` for: Non-fatal issues requiring attention; Deprecated functionality usage; Configuration problems that don't block execution -- Use `Write-Error` for most error scenarios in public commands +- **Prefer `Write-Error` for public commands** - provides standard PowerShell non-terminating error behavior; callers control termination via `-ErrorAction 'Stop'` - Always include `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), `-TargetObject` (object causing error) - In catch blocks, pass original exception using `-Exception` - Always use `return` after `Write-Error` to avoid further processing -- Use `$PSCmdlet.ThrowTerminatingError()` only in limited scenarios: assert-style commands, private functions, or catch blocks; note that callers must use `-ErrorAction 'Stop'` to catch these errors -- Use `throw` only in `[ValidateScript()]` parameter validation attributes +- **Avoid `$PSCmdlet.ThrowTerminatingError()` in public commands** - creates command-terminating (not script-terminating) errors; callers must set `$ErrorActionPreference = 'Stop'` OR use try-catch (using `-ErrorAction 'Stop'` alone is insufficient) + - Acceptable only in: assert-style commands, private functions, or state-changing catch blocks where operation failures must prevent further state changes +- Use `throw` only in `[ValidateScript()]` parameter validation attributes - it's the only valid mechanism there - .NET method exceptions (e.g., SMO methods) are always caught in try-catch blocks without needing to set `$ErrorActionPreference` ## ShouldProcess Required Pattern diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec5f2b410d..3c8da5ce83 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -419,8 +419,11 @@ Use `$PSCmdlet.ThrowTerminatingError()` only in these limited scenarios: failure must be communicated as a terminating error to prevent further state changes -In these cases, ensure callers are aware of the behavior and use -`-ErrorAction 'Stop'` when calling the command from other commands. +In these cases, ensure callers are aware of the behavior. **Important:** When calling +these commands, using `-ErrorAction 'Stop'` alone is NOT sufficient to stop the calling +function. Callers must ALSO either: +- Wrap the call in try-catch, OR +- Set `$ErrorActionPreference = 'Stop'` in the calling function ##### Handling Errors from .NET Methods and Cmdlets @@ -498,9 +501,16 @@ finally > [!NOTE] > Setting `$ErrorActionPreference = 'Stop'` is particularly useful when calling -> commands that use `$PSCmdlet.ThrowTerminatingError()`. Without it (or -> `-ErrorAction 'Stop'` on each call), these command-terminating errors will not -> stop the calling function - it will continue executing after the error. +> commands that use `$PSCmdlet.ThrowTerminatingError()`. **Important:** +> Using `-ErrorAction 'Stop'` alone when calling such commands is NOT sufficient +> to stop the calling function - it will continue executing after the error. +> You must set `$ErrorActionPreference = 'Stop'` OR wrap the call in try-catch. + +> [!TIP] +> **Prefer `Write-Error` in public commands instead of `$PSCmdlet.ThrowTerminatingError()`.** +> With `Write-Error`, callers can use `-ErrorAction 'Stop'` alone and it works as +> expected - the caller stops executing. This is standard PowerShell behavior and +> avoids the need for `$ErrorActionPreference` or try-catch in callers. > [!IMPORTANT] > **Do not** use the pattern `$ErrorActionPreference = 'Stop'` followed by @@ -602,7 +612,7 @@ $results = $items | Process-Items -ErrorAction 'Stop' | Catching .NET method exceptions | try-catch without setting `$ErrorActionPreference` | .NET exceptions are always caught automatically | | Blanket error handling for multiple cmdlets | Set `$ErrorActionPreference = 'Stop'` in try, restore in finally | Avoids adding `-ErrorAction 'Stop'` to each cmdlet; also catches ThrowTerminatingError from child commands | | Single cmdlet error handling | Use `-ErrorAction 'Stop'` on the cmdlet | Simpler and more explicit than setting `$ErrorActionPreference` | -| Calling commands that use ThrowTerminatingError | Set `$ErrorActionPreference = 'Stop'` or use `-ErrorAction 'Stop'` | Required to catch these command-terminating errors; without it, caller continues after error | +| Calling commands that use ThrowTerminatingError | Set `$ErrorActionPreference = 'Stop'` in caller OR wrap in try-catch | Using `-ErrorAction 'Stop'` alone is NOT sufficient; caller continues after error | | Assert-style commands | `$PSCmdlet.ThrowTerminatingError()` | Command purpose is to throw on failure | | State-changing commands (catch blocks) | `$PSCmdlet.ThrowTerminatingError()` | Prevents partial state changes; caller must use `-ErrorAction 'Stop'` or set `$ErrorActionPreference` | | Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | From 78e81ecab3a80b4f35a5fae83902f83df9dbf38b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 17:39:00 +0000 Subject: [PATCH 09/15] Fix inconsistency in state-changing commands table row Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3c8da5ce83..1c13ac591b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -614,7 +614,7 @@ $results = $items | Process-Items -ErrorAction 'Stop' | Single cmdlet error handling | Use `-ErrorAction 'Stop'` on the cmdlet | Simpler and more explicit than setting `$ErrorActionPreference` | | Calling commands that use ThrowTerminatingError | Set `$ErrorActionPreference = 'Stop'` in caller OR wrap in try-catch | Using `-ErrorAction 'Stop'` alone is NOT sufficient; caller continues after error | | Assert-style commands | `$PSCmdlet.ThrowTerminatingError()` | Command purpose is to throw on failure | -| State-changing commands (catch blocks) | `$PSCmdlet.ThrowTerminatingError()` | Prevents partial state changes; caller must use `-ErrorAction 'Stop'` or set `$ErrorActionPreference` | +| State-changing commands (catch blocks) | `$PSCmdlet.ThrowTerminatingError()` | Prevents partial state changes; caller must set `$ErrorActionPreference = 'Stop'` OR wrap in try-catch | | Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | | Parameter validation in `[ValidateScript()]` | `throw` | Only valid option within validation attributes | | Any other scenario in commands | Never use `throw` | Poor error messages; unpredictable behavior | From 95ddc8d9863a46ce7c3a7d7f81c9777e9abc5f10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 17:39:56 +0000 Subject: [PATCH 10/15] Update CHANGELOG with concise error handling changes Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d23f5d0546..6b995bc292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,8 +59,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Set-SqlDscDatabaseProperty` - Updated comment-based help to reference correct enum values. - Added SQL Server version requirements to version-specific parameter help. -- Updated CONTRIBUTING.md error handling documentation to clarify proper usage - of `Write-Error` vs `$PSCmdlet.ThrowTerminatingError()` in public commands +- Updated CONTRIBUTING.md error handling guidelines: recommend `Write-Error` for + public commands; clarified that `-ErrorAction 'Stop'` alone is insufficient + when calling commands using `$PSCmdlet.ThrowTerminatingError()` - callers must + set `$ErrorActionPreference = 'Stop'` or use try-catch ([issue #2193](https://github.com/dsccommunity/SqlServerDsc/issues/2193)). ### Fixed From 70d34ca4c53547c04c8356e566b8475b5286bed8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 17:42:15 +0000 Subject: [PATCH 11/15] Fix inconsistencies in error handling documentation Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c13ac591b..a58124b0e0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -365,8 +365,10 @@ throws a **command-terminating error** which has unexpected behavior when called from another command. **The Problem**: When a command uses `$PSCmdlet.ThrowTerminatingError()`, the -error behaves like a non-terminating error to the caller unless the caller -explicitly uses `-ErrorAction 'Stop'`. This creates confusing behavior: +error is command-terminating (stops that command) but NOT script-terminating. The +calling function continues executing unless the caller sets +`$ErrorActionPreference = 'Stop'` or wraps the call in try-catch. This creates +confusing behavior: ```powershell function Get-Something @@ -398,8 +400,8 @@ function Start-Operation Write-Output 'Operation completed' } -# Caller must use -ErrorAction 'Stop' to prevent continued execution -Start-Operation -ErrorAction 'Stop' +# Caller must set ErrorActionPreference = 'Stop' or use try-catch to prevent continued execution +Start-Operation -ErrorAction 'Stop' # This alone is NOT sufficient! ``` Because of this behavior, **use `Write-Error` instead** in public commands. @@ -444,8 +446,9 @@ how exceptions are caught: This example shows a state-changing command using `$PSCmdlet.ThrowTerminatingError()` in a catch block. This is acceptable for commands with `SupportsShouldProcess` where -operation failures must terminate. Note that callers must use `-ErrorAction 'Stop'` -to catch this error. +operation failures must terminate. **Important:** Callers must set +`$ErrorActionPreference = 'Stop'` or wrap the call in try-catch to stop execution +on error (using `-ErrorAction 'Stop'` alone is insufficient). For non-state-changing commands (like Get/Test commands), prefer `Write-Error` in catch blocks instead. From 586fd94e2f65985b248967fa47418a3633db6f9b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 17:58:37 +0000 Subject: [PATCH 12/15] Remove assert-style commands from acceptable ThrowTerminatingError uses Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- ...munity-style-guidelines-powershell.instructions.md | 3 ++- CONTRIBUTING.md | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md index bd35914ea5..cf593d2c85 100644 --- a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md +++ b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md @@ -98,7 +98,8 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - In catch blocks, pass original exception using `-Exception` - Always use `return` after `Write-Error` to avoid further processing - **Avoid `$PSCmdlet.ThrowTerminatingError()` in public commands** - creates command-terminating (not script-terminating) errors; callers must set `$ErrorActionPreference = 'Stop'` OR use try-catch (using `-ErrorAction 'Stop'` alone is insufficient) - - Acceptable only in: assert-style commands, private functions, or state-changing catch blocks where operation failures must prevent further state changes + - Acceptable only in: private functions, or state-changing catch blocks where operation failures must prevent further state changes + - Assert-style commands should use `Write-Error` for standard behavior - Use `throw` only in `[ValidateScript()]` parameter validation attributes - it's the only valid mechanism there - .NET method exceptions (e.g., SMO methods) are always caught in try-catch blocks without needing to set `$ErrorActionPreference` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a58124b0e0..1b1a9d1fff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -412,15 +412,16 @@ Use `$PSCmdlet.ThrowTerminatingError()` only in these limited scenarios: 1. **Private functions** that are only called internally and where the behavior is well understood -2. **Assert-style commands** (like `Assert-SqlDscLogin`) whose purpose is to - throw on failure -3. **Commands with `SupportsShouldProcess`** where an operation failure in the +2. **Commands with `SupportsShouldProcess`** where an operation failure in the `ShouldProcess` block must terminate (common for state-changing operations that cannot be partially completed) -4. **Catch blocks in state-changing commands** where a critical operation +3. **Catch blocks in state-changing commands** where a critical operation failure must be communicated as a terminating error to prevent further state changes +**Note:** Assert-style commands should use `Write-Error` instead - this provides +standard PowerShell behavior where `-ErrorAction 'Stop'` alone works correctly. + In these cases, ensure callers are aware of the behavior. **Important:** When calling these commands, using `-ErrorAction 'Stop'` alone is NOT sufficient to stop the calling function. Callers must ALSO either: @@ -616,7 +617,7 @@ $results = $items | Process-Items -ErrorAction 'Stop' | Blanket error handling for multiple cmdlets | Set `$ErrorActionPreference = 'Stop'` in try, restore in finally | Avoids adding `-ErrorAction 'Stop'` to each cmdlet; also catches ThrowTerminatingError from child commands | | Single cmdlet error handling | Use `-ErrorAction 'Stop'` on the cmdlet | Simpler and more explicit than setting `$ErrorActionPreference` | | Calling commands that use ThrowTerminatingError | Set `$ErrorActionPreference = 'Stop'` in caller OR wrap in try-catch | Using `-ErrorAction 'Stop'` alone is NOT sufficient; caller continues after error | -| Assert-style commands | `$PSCmdlet.ThrowTerminatingError()` | Command purpose is to throw on failure | +| Assert-style commands | `Write-Error` | Provides standard behavior; `-ErrorAction 'Stop'` alone works correctly for callers | | State-changing commands (catch blocks) | `$PSCmdlet.ThrowTerminatingError()` | Prevents partial state changes; caller must set `$ErrorActionPreference = 'Stop'` OR wrap in try-catch | | Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | | Parameter validation in `[ValidateScript()]` | `throw` | Only valid option within validation attributes | From b3ad133c6726387cba8337bfc2915b48440f7031 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 18:08:45 +0000 Subject: [PATCH 13/15] Simplify error handling documentation - use Write-Error with -ErrorAction Stop Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- ...tyle-guidelines-powershell.instructions.md | 15 +- CONTRIBUTING.md | 268 +++++++----------- 2 files changed, 103 insertions(+), 180 deletions(-) diff --git a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md index cf593d2c85..3b4613f1c7 100644 --- a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md +++ b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md @@ -82,8 +82,6 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - `$PSCmdlet.ShouldProcess` must use required pattern - Inside `$PSCmdlet.ShouldProcess`-block, avoid using `Write-Verbose` - Never use backtick as line continuation in production code. -- For blanket error handling across multiple cmdlets, set `$ErrorActionPreference = 'Stop'` in try block and restore in finally block -- When calling commands that use `$PSCmdlet.ThrowTerminatingError()`, set `$ErrorActionPreference = 'Stop'` OR wrap in try-catch (using `-ErrorAction 'Stop'` alone is insufficient) - Use `[Alias()]` attribute for function aliases, never `Set-Alias` or `New-Alias` ## Output streams @@ -93,15 +91,16 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - Use `Write-Verbose` for: High-level execution flow only; User-actionable information - Use `Write-Information` for: User-facing status updates; Important operational messages; Non-error state changes - Use `Write-Warning` for: Non-fatal issues requiring attention; Deprecated functionality usage; Configuration problems that don't block execution -- **Prefer `Write-Error` for public commands** - provides standard PowerShell non-terminating error behavior; callers control termination via `-ErrorAction 'Stop'` +- **Use `Write-Error` for all error handling in public commands** + - For terminating errors: Add `-ErrorAction 'Stop'` parameter to `Write-Error` + - For non-terminating errors: Omit `-ErrorAction` parameter (caller controls via `-ErrorAction`) - Always include `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), `-TargetObject` (object causing error) - In catch blocks, pass original exception using `-Exception` - Always use `return` after `Write-Error` to avoid further processing -- **Avoid `$PSCmdlet.ThrowTerminatingError()` in public commands** - creates command-terminating (not script-terminating) errors; callers must set `$ErrorActionPreference = 'Stop'` OR use try-catch (using `-ErrorAction 'Stop'` alone is insufficient) - - Acceptable only in: private functions, or state-changing catch blocks where operation failures must prevent further state changes - - Assert-style commands should use `Write-Error` for standard behavior -- Use `throw` only in `[ValidateScript()]` parameter validation attributes - it's the only valid mechanism there -- .NET method exceptions (e.g., SMO methods) are always caught in try-catch blocks without needing to set `$ErrorActionPreference` +- **Never use `$PSCmdlet.ThrowTerminatingError()` in public commands** - it creates command-terminating (not script-terminating) errors; use `Write-Error` with `-ErrorAction 'Stop'` instead + - May be used in private functions where behavior is understood by internal callers +- **Never use `throw` in public commands** except in `[ValidateScript()]` parameter validation attributes (it's the only valid mechanism there) +- .NET method exceptions (e.g., SMO methods) are always caught in try-catch blocks - no special handling needed ## ShouldProcess Required Pattern diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1b1a9d1fff..8c339e5bda 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -357,221 +357,149 @@ $db = Get-Database -Name 'NonExistent' -ErrorAction 'Stop' > command. Without `return`, the command continues executing, which may cause > unexpected behavior or additional errors. -##### Understanding $PSCmdlet.ThrowTerminatingError +##### Error Handling in Public Commands -The `$PSCmdlet.ThrowTerminatingError()` method is **not recommended** for public -commands despite its name suggesting it throws a terminating error. It actually -throws a **command-terminating error** which has unexpected behavior when called -from another command. - -**The Problem**: When a command uses `$PSCmdlet.ThrowTerminatingError()`, the -error is command-terminating (stops that command) but NOT script-terminating. The -calling function continues executing unless the caller sets -`$ErrorActionPreference = 'Stop'` or wraps the call in try-catch. This creates -confusing behavior: +**Always use `Write-Error` in public commands.** For errors that should terminate execution: ```powershell -function Get-Something +function Get-Database { [CmdletBinding()] - param () - - $PSCmdlet.ThrowTerminatingError( - [System.Management.Automation.ErrorRecord]::new( - 'Database not found', - 'GS0001', - [System.Management.Automation.ErrorCategory]::ObjectNotFound, - 'MyDatabase' - ) - ) - - # This line never executes (correctly stopped) - Write-Output 'After error' -} + param ([Parameter(Mandatory = $true)] [System.String] $Name) -function Start-Operation -{ - [CmdletBinding()] - param () - - Get-Something + if (-not (Test-DatabaseExists $Name)) + { + Write-Error -Message "Database '$Name' not found" ` + -Category ObjectNotFound ` + -ErrorId 'GD0001' ` + -TargetObject $Name ` + -ErrorAction 'Stop' + + return + } - # BUG: This line DOES execute even though Get-Something threw an error! - Write-Output 'Operation completed' + # Continue processing... } - -# Caller must set ErrorActionPreference = 'Stop' or use try-catch to prevent continued execution -Start-Operation -ErrorAction 'Stop' # This alone is NOT sufficient! ``` -Because of this behavior, **use `Write-Error` instead** in public commands. - -##### When $PSCmdlet.ThrowTerminatingError Can Be Used - -Use `$PSCmdlet.ThrowTerminatingError()` only in these limited scenarios: - -1. **Private functions** that are only called internally and where the behavior - is well understood -2. **Commands with `SupportsShouldProcess`** where an operation failure in the - `ShouldProcess` block must terminate (common for state-changing operations - that cannot be partially completed) -3. **Catch blocks in state-changing commands** where a critical operation - failure must be communicated as a terminating error to prevent further - state changes +With this pattern: +- The error terminates execution (stops the function and the caller) +- Callers don't need special error handling +- Standard PowerShell behavior - simple and predictable -**Note:** Assert-style commands should use `Write-Error` instead - this provides -standard PowerShell behavior where `-ErrorAction 'Stop'` alone works correctly. +##### Why Not $PSCmdlet.ThrowTerminatingError? -In these cases, ensure callers are aware of the behavior. **Important:** When calling -these commands, using `-ErrorAction 'Stop'` alone is NOT sufficient to stop the calling -function. Callers must ALSO either: -- Wrap the call in try-catch, OR -- Set `$ErrorActionPreference = 'Stop'` in the calling function +The `$PSCmdlet.ThrowTerminatingError()` method creates **command-terminating** errors, +not script-terminating errors. The calling function continues executing after the error +unless the caller uses `$ErrorActionPreference = 'Stop'` or try-catch: -##### Handling Errors from .NET Methods and Cmdlets - -When working with .NET methods (like SMO objects) and PowerShell cmdlets, understand -how exceptions are caught: +```powershell +function Get-Something +{ + $PSCmdlet.ThrowTerminatingError(...) # Stops Get-Something +} -**For .NET methods (.NET Framework/Core APIs and SMO):** -- Exceptions are **always caught** in try-catch blocks -- No need to set `$ErrorActionPreference` - the exceptions throw naturally -- Example: `$alertObject.Drop()`, `$database.Create()`, `[System.IO.File]::OpenRead()` +function Start-Operation +{ + Get-Something # Error occurs + Write-Output 'Continues' # BUG: This executes! +} +``` -**For PowerShell cmdlets:** -- Use `-ErrorAction 'Stop'` to make errors catchable in try-catch -- **Do not** set `$ErrorActionPreference = 'Stop'` when using `-ErrorAction 'Stop'` - it's redundant -- Only set `$ErrorActionPreference = 'Stop'` if you need blanket error handling for multiple cmdlets +**Do not use `$PSCmdlet.ThrowTerminatingError()` in public commands.** It may be used +in private functions where the behavior is well understood by internal callers only. -**Recommended pattern for .NET methods:** +##### Exception Handling in Commands -This example shows a state-changing command using `$PSCmdlet.ThrowTerminatingError()` -in a catch block. This is acceptable for commands with `SupportsShouldProcess` where -operation failures must terminate. **Important:** Callers must set -`$ErrorActionPreference = 'Stop'` or wrap the call in try-catch to stop execution -on error (using `-ErrorAction 'Stop'` alone is insufficient). +When catching exceptions in try-catch blocks, use `Write-Error` with the original exception. -For non-state-changing commands (like Get/Test commands), prefer `Write-Error` in -catch blocks instead. +**For terminating errors** (execution should stop): ```powershell try { - # .NET methods throw exceptions automatically - no ErrorActionPreference needed - $alertObject.Drop() - - Write-Verbose -Message ($script:localizedData.AlertRemoved -f $alertObject.Name) + $database.Create() } catch { - $errorMessage = $script:localizedData.RemoveFailed -f $alertObject.Name - - $PSCmdlet.ThrowTerminatingError( - [System.Management.Automation.ErrorRecord]::new( - [System.InvalidOperationException]::new($errorMessage, $_.Exception), - 'RSAA0001', - [System.Management.Automation.ErrorCategory]::InvalidOperation, - $alertObject - ) - ) + $errorMessage = $script:localizedData.CreateDatabaseFailed -f $DatabaseName + + Write-Error -Message $errorMessage ` + -Category 'InvalidOperation' ` + -ErrorId 'CD0001' ` + -TargetObject $DatabaseName ` + -Exception $_.Exception ` + -ErrorAction 'Stop' + + return } ``` -**Pattern for blanket error handling:** - -Use `$ErrorActionPreference = 'Stop'` when you need blanket error handling for -multiple cmdlets or when calling commands that use `$PSCmdlet.ThrowTerminatingError()`: +**For non-terminating errors** (allow caller to control): ```powershell try { - $originalErrorActionPreference = $ErrorActionPreference - $ErrorActionPreference = 'Stop' - - # Multiple cmdlets without -ErrorAction on each - $instance = Get-SqlDscManagedComputerInstance -InstanceName $InstanceName - $protocol = Get-SqlDscServerProtocolName -ProtocolName $ProtocolName - $serverProtocol = $instance.ServerProtocols[$protocol.ShortName] + $database.Create() } catch { - # Handle error -} -finally -{ - $ErrorActionPreference = $originalErrorActionPreference + $errorMessage = $script:localizedData.CreateDatabaseFailed -f $DatabaseName + + Write-Error -Message $errorMessage ` + -Category 'InvalidOperation' ` + -ErrorId 'CD0001' ` + -TargetObject $DatabaseName ` + -Exception $_.Exception + + return } ``` -> [!NOTE] -> Setting `$ErrorActionPreference = 'Stop'` is particularly useful when calling -> commands that use `$PSCmdlet.ThrowTerminatingError()`. **Important:** -> Using `-ErrorAction 'Stop'` alone when calling such commands is NOT sufficient -> to stop the calling function - it will continue executing after the error. -> You must set `$ErrorActionPreference = 'Stop'` OR wrap the call in try-catch. +##### .NET Method Exceptions -> [!TIP] -> **Prefer `Write-Error` in public commands instead of `$PSCmdlet.ThrowTerminatingError()`.** -> With `Write-Error`, callers can use `-ErrorAction 'Stop'` alone and it works as -> expected - the caller stops executing. This is standard PowerShell behavior and -> avoids the need for `$ErrorActionPreference` or try-catch in callers. +.NET methods (like SMO objects) throw exceptions automatically - no special error +handling needed. Just use try-catch: -> [!IMPORTANT] -> **Do not** use the pattern `$ErrorActionPreference = 'Stop'` followed by -> `-ErrorAction 'Stop'` on the same cmdlet - this is redundant. The -> `-ErrorAction` parameter takes precedence over `$ErrorActionPreference` -> for that specific cmdlet call. - -> [!NOTE] -> The pattern of setting `$ErrorActionPreference = 'Stop'` in a try block -> with restoration in finally is useful for: -> - Blanket error handling across multiple cmdlets -> - Catching errors from commands that use `$PSCmdlet.ThrowTerminatingError()` -> (without this, those errors won't stop the caller) -> - However, it's NOT necessary for .NET method calls (they always throw) or -> when using `-ErrorAction 'Stop'` on individual cmdlets. +```powershell +try +{ + $alertObject.Drop() # Throws exception on failure + Write-Verbose -Message ($script:localizedData.AlertRemoved -f $alertObject.Name) +} +catch +{ + $errorMessage = $script:localizedData.RemoveFailed -f $alertObject.Name + + Write-Error -Message $errorMessage ` + -Category 'InvalidOperation' ` + -ErrorId 'RMA0001' ` + -TargetObject $alertObject ` + -Exception $_.Exception ` + -ErrorAction 'Stop' + + return +} +``` ##### Parameter Validation with ValidateScript -When using `[ValidateScript()]` attribute on parameters, you **must** use -`throw` for validation failures, as it is the only mechanism that works -within validation attributes: +When using `[ValidateScript()]` attribute, use `throw` for validation failures +(this is the only mechanism that works within validation attributes): ```powershell [Parameter()] [ValidateScript({ if (-not (Test-Path -Path $_)) { - throw ($script:localizedData.Audit_PathParameterValueInvalid -f $_) + throw ($script:localizedData.PathParameterInvalid -f $_) } - + return $true })] [System.String] $Path ``` - -This is the **only acceptable use** of `throw` in public commands and private -functions. - -##### Exception Handling in Commands - -When catching exceptions in try-catch blocks, re-throw errors using `Write-Error` -with the original exception: - -```powershell -try -{ - $database.Create() -} -catch -{ - $errorMessage = $script:localizedData.CreateDatabaseFailed -f $DatabaseName - - Write-Error -Message $errorMessage -Category 'InvalidOperation' -ErrorId 'CD0001' -TargetObject $DatabaseName -Exception $_.Exception - - return -} ``` ##### Pipeline Processing Considerations @@ -609,17 +537,13 @@ $results = $items | Process-Items -ErrorAction 'Stop' ##### Summary -| Scenario | Use | Why | -|----------|-----|-----| -| General error handling in public commands | `Write-Error` | Provides consistent, predictable behavior; allows caller to control termination | -| Pipeline processing with multiple items | `Write-Error` | Allows processing to continue for remaining items | -| Catching .NET method exceptions | try-catch without setting `$ErrorActionPreference` | .NET exceptions are always caught automatically | -| Blanket error handling for multiple cmdlets | Set `$ErrorActionPreference = 'Stop'` in try, restore in finally | Avoids adding `-ErrorAction 'Stop'` to each cmdlet; also catches ThrowTerminatingError from child commands | -| Single cmdlet error handling | Use `-ErrorAction 'Stop'` on the cmdlet | Simpler and more explicit than setting `$ErrorActionPreference` | -| Calling commands that use ThrowTerminatingError | Set `$ErrorActionPreference = 'Stop'` in caller OR wrap in try-catch | Using `-ErrorAction 'Stop'` alone is NOT sufficient; caller continues after error | -| Assert-style commands | `Write-Error` | Provides standard behavior; `-ErrorAction 'Stop'` alone works correctly for callers | -| State-changing commands (catch blocks) | `$PSCmdlet.ThrowTerminatingError()` | Prevents partial state changes; caller must set `$ErrorActionPreference = 'Stop'` OR wrap in try-catch | -| Private functions (internal use only) | `$PSCmdlet.ThrowTerminatingError()` or `Write-Error` | Behavior is understood by internal callers | +| Scenario | Use | Notes | +|----------|-----|-------| +| Terminating errors in public commands | `Write-Error` with `-ErrorAction 'Stop'` | Simple and standard PowerShell behavior | +| Non-terminating errors in public commands | `Write-Error` without `-ErrorAction` | Allows caller to control termination via `-ErrorAction` | +| Pipeline processing with multiple items | `Write-Error` without `-ErrorAction` | Allows processing to continue for remaining items | +| Catching .NET method exceptions | try-catch with `Write-Error` | .NET exceptions are always caught automatically | | Parameter validation in `[ValidateScript()]` | `throw` | Only valid option within validation attributes | -| Any other scenario in commands | Never use `throw` | Poor error messages; unpredictable behavior | +| Private functions (internal use only) | `Write-Error` or `$PSCmdlet.ThrowTerminatingError()` | Behavior is understood by internal callers | +| Public commands | Never use `throw` or `$PSCmdlet.ThrowTerminatingError()` | Use `Write-Error` instead | From 026f90fe5416bfa73504bb5d18b9529362483ee9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 18:19:20 +0000 Subject: [PATCH 14/15] Remove duplicate .NET section and unnecessary return after -ErrorAction Stop Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- CONTRIBUTING.md | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8c339e5bda..c5fcbd0812 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -430,8 +430,6 @@ catch -TargetObject $DatabaseName ` -Exception $_.Exception ` -ErrorAction 'Stop' - - return } ``` @@ -456,32 +454,6 @@ catch } ``` -##### .NET Method Exceptions - -.NET methods (like SMO objects) throw exceptions automatically - no special error -handling needed. Just use try-catch: - -```powershell -try -{ - $alertObject.Drop() # Throws exception on failure - Write-Verbose -Message ($script:localizedData.AlertRemoved -f $alertObject.Name) -} -catch -{ - $errorMessage = $script:localizedData.RemoveFailed -f $alertObject.Name - - Write-Error -Message $errorMessage ` - -Category 'InvalidOperation' ` - -ErrorId 'RMA0001' ` - -TargetObject $alertObject ` - -Exception $_.Exception ` - -ErrorAction 'Stop' - - return -} -``` - ##### Parameter Validation with ValidateScript When using `[ValidateScript()]` attribute, use `throw` for validation failures @@ -500,7 +472,6 @@ When using `[ValidateScript()]` attribute, use `throw` for validation failures [System.String] $Path ``` -``` ##### Pipeline Processing Considerations From 92489fc32f40d56e4b5e228d650d50988d378287 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 18:41:02 +0000 Subject: [PATCH 15/15] Clarify return usage with Write-Error and fix markdown table formatting Co-authored-by: johlju <7189721+johlju@users.noreply.github.com> --- ...unity-style-guidelines-powershell.instructions.md | 2 +- CONTRIBUTING.md | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md index 3b4613f1c7..cc57ce5613 100644 --- a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md +++ b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md @@ -96,7 +96,7 @@ applyTo: "{**/*.ps1,**/*.psm1,**/*.psd1}" - For non-terminating errors: Omit `-ErrorAction` parameter (caller controls via `-ErrorAction`) - Always include `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), `-TargetObject` (object causing error) - In catch blocks, pass original exception using `-Exception` - - Always use `return` after `Write-Error` to avoid further processing + - Use `return` only after non-terminating `Write-Error` to stop further processing. Omit `return` when using `-ErrorAction 'Stop'`. - **Never use `$PSCmdlet.ThrowTerminatingError()` in public commands** - it creates command-terminating (not script-terminating) errors; use `Write-Error` with `-ErrorAction 'Stop'` instead - May be used in private functions where behavior is understood by internal callers - **Never use `throw` in public commands** except in `[ValidateScript()]` parameter validation attributes (it's the only valid mechanism there) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c5fcbd0812..cb2ba7cb61 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -353,9 +353,9 @@ $db = Get-Database -Name 'NonExistent' -ErrorAction 'Stop' ``` > [!IMPORTANT] -> Always use `return` after `Write-Error` to prevent further processing in the -> command. Without `return`, the command continues executing, which may cause -> unexpected behavior or additional errors. +> Use `return` after `Write-Error` only for non-terminating errors (no `-ErrorAction 'Stop'`) +> to stop further processing in the current function. Omit `return` when using +> `-ErrorAction 'Stop'`, as execution stops automatically. ##### Error Handling in Public Commands @@ -374,8 +374,6 @@ function Get-Database -ErrorId 'GD0001' ` -TargetObject $Name ` -ErrorAction 'Stop' - - return } # Continue processing... @@ -507,9 +505,8 @@ $results = $items | Process-Items -ErrorAction 'Stop' ##### Summary - | Scenario | Use | Notes | -|----------|-----|-------| +| --- | --- | --- | | Terminating errors in public commands | `Write-Error` with `-ErrorAction 'Stop'` | Simple and standard PowerShell behavior | | Non-terminating errors in public commands | `Write-Error` without `-ErrorAction` | Allows caller to control termination via `-ErrorAction` | | Pipeline processing with multiple items | `Write-Error` without `-ErrorAction` | Allows processing to continue for remaining items | @@ -517,4 +514,3 @@ $results = $items | Process-Items -ErrorAction 'Stop' | Parameter validation in `[ValidateScript()]` | `throw` | Only valid option within validation attributes | | Private functions (internal use only) | `Write-Error` or `$PSCmdlet.ThrowTerminatingError()` | Behavior is understood by internal callers | | Public commands | Never use `throw` or `$PSCmdlet.ThrowTerminatingError()` | Use `Write-Error` instead | -