Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 7, 2025

  • Update AI instructions: clarify return only needed for non-terminating Write-Error
  • Update CONTRIBUTING.md: clarify return usage and remove it from -ErrorAction Stop example
  • Fix markdown table formatting: remove leading/trailing pipes to satisfy MD055
  • All code review feedback addressed
Original prompt

This section details on the original issue you should resolve

<issue_title>CONTRIBUTING: Update information around $PSCmdlet.ThrowTerminatingError</issue_title>
<issue_description>The documentation should be updated according to this: #1966 (comment)

Also reference: https://stackoverflow.com/questions/49204918/difference-between-throw-and-pscmdlet-throwterminatingerror

When instead using `$PSCmdlet.ThrowTerminatingError()`:
```powershell
$PSCmdlet.ThrowTerminatingError(
[System.Management.Automation.ErrorRecord]::new(
'MyError',
'GS0001',
[System.Management.Automation.ErrorCategory]::InvalidOperation,
'MyObjectOrValue'
)
)
```

I caught this today where I call a public command (or function) from a another public command. Example below.

function Get-Something
{
    [CmdletBinding()]
    param ()

    $PSCmdlet.ThrowTerminatingError(
        [System.Management.Automation.ErrorRecord]::new(
            'Error message',
            'CODE',
            [System.Management.Automation.ErrorCategory]::InvalidOperation,
            'MyObject'
        )
    )

    "Get-Something exiting" # CORRECT: Does not hit this
}

function Start-Something
{
    [CmdletBinding()]
    param ()

    Get-Something -Name $null -ErrorAction 'Stop'

    "Started" # BUG: This line is executed even though Get-Something throw an exception
}

# This hits the bug in Start-Something
Start-Something

# The user must add -ErrorAction 'Stop' to Start-Something to avoid the bug which is not intuitive
Start-Something -ErrorAction 'Stop'

Similar but using Write-Error (non-terminating error):

function Get-Something
{
    [CmdletBinding()]
    param ()

    Write-Error -Message 'Error message' -Category InvalidOperation -TargetObject 'MyObject' -ErrorId 'CODE'

    "Get-Something exiting" # CORRECT: Does not hit this
}

function Start-Something
{
    [CmdletBinding()]
    param ()

    Get-Something -Name $null -ErrorAction 'Stop'

    "Started" # CORRECT: Does not hit this
}

# This works as expected, stops after the exception in Get-Something
Start-Something
```</issue_description>

<agent_instructions>Read the issue and comment, follow the links and read those comments and links. Based on that make a conclusion about how to best use different ways of throwing a terminating and non-terminating error in public commands and update CONTRIBUTING.md. We should never use `throw` in public commands though, so that is not in scope. My basic understanding is that we should use `Write-Error` whenever possible, but should we limit or entirely not use `$PSCmdlet.ThrowTerminatingError()`? What scenarios should we use each in, and if possible give examples of scenarios where each option fail and where they succeed. We have scenarios like commands accepting pipeline input (that process several items at a time over pipeline), not accepting pipeline input, commands that should never throw a terminating error by default but a non-terminating error and commands that in certain code paths must throw a terminating error.</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


This change is Reviewable

@coderabbitai
Copy link

coderabbitai bot commented Dec 7, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This PR updates PowerShell community style guidelines and contribution documentation to standardize error handling practices in public commands, replacing prior guidance with mandatory Write-Error usage instead of $PSCmdlet.ThrowTerminatingError(), and corrects a typo in documented command names.

Changes

Cohort / File(s) Summary
Error Handling Guidelines
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md, CONTRIBUTING.md
Replaces error handling and ShouldProcess guidance with comprehensive rules requiring Write-Error for all public command errors, prohibiting $PSCmdlet.ThrowTerminatingError() and throw statements in public commands (except parameter validation), and specifying mandatory error record structure with -Message, -Category, -ErrorId, and -TargetObject.
Documentation Updates
CHANGELOG.md
Updates changelog to reference the new error handling guidelines and issue #2193.
Typo Corrections
CONTRIBUTING.md
Fixes misspelled command name New-InvalidDataExcpetion to New-InvalidDataException and corrects additional typographical errors in documentation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring attention:
    • Verify the new Write-Error guidelines align with PowerShell best practices and the linked issues' requirements
    • Review the distinction between terminating and non-terminating error handling patterns with respect to -ErrorAction behavior
    • Confirm that the prohibition on $PSCmdlet.ThrowTerminatingError() in public commands is intentional and consistent across guidance

Possibly related PRs

Pre-merge checks

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main change: simplifying error handling guidelines to recommend using Write-Error instead of ThrowTerminatingError in public commands.
Description check ✅ Passed The description comprehensively explains the problem, solution, and testing validation, providing clear context for the error handling simplification and changes across documentation files.
Linked Issues check ✅ Passed The PR addresses both linked issues by simplifying error handling documentation and recommending Write-Error with -ErrorAction 'Stop' for terminating errors, directly resolving the core concern raised in #2045 about ThrowTerminatingError propagation behavior.
Out of Scope Changes check ✅ Passed All changes are scoped to error handling documentation and related AI instructions; there are no code changes to actual commands or unrelated modifications, and the scope remains focused on the linked issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI changed the title [WIP] Update documentation for pscmdlet.throwterminatingerror CONTRIBUTING: Update error handling guidelines for public commands Dec 7, 2025
Copilot AI requested a review from johlju December 7, 2025 14:17
@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot there are more scenarios that need to be documented:

  1. There is an exception to not use throw , the exception is when a [ValidateScript()]on a parameter in a private function or public command has to throw an error, thenthrow` must be used.
  2. There is scenario in the codebase that set ErrorActionPreference = 'Stop' prior to calling a command with -ErrorAction 'Stop' to get the parent caller to stop on terminating error (and reverting ErrorActionPreference after the call). See the repository AI instructions that also mentioned this (which might be an error in the AI instructions) Is that an issue because $PSCmdLet.ThrowTerminationError() is used in the called command, or is there a scenarion where this is required. Analyze the codebase to see a pattern and if that pattern is required.

Copy link
Contributor Author

Copilot AI commented Dec 7, 2025

@copilot there are more scenarios that need to be documented:

  1. There is an exception to not use throw , the exception is when a [ValidateScript()]` on a parameter in a private function or publi...

Both scenarios are now documented in commit e24f11c:

  1. ValidateScript exception: Added documentation showing that throw is the only valid option within [ValidateScript()] parameter validation attributes.

  2. ErrorActionPreference pattern: After testing, I found that setting $ErrorActionPreference = 'Stop' is NOT needed in most cases:

    • ✅ .NET method exceptions (SMO methods like .Drop()) are always caught automatically
    • ✅ Use -ErrorAction 'Stop' on individual cmdlets (don't also set ErrorActionPreference - it's redundant)
    • ✅ Only set $ErrorActionPreference = 'Stop' for blanket error handling across multiple cmdlets without adding -ErrorAction to each

The AI instruction line 85 is incorrect and should be updated. The documentation now clarifies when each pattern is appropriate.

@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot In the example shows how a state-changing command is using $PSCmdlet.ThrowTerminatingError(), is there a possible solution for where a user does not need to pass -ErrorAction 'Stop' when calling a state-changing public command and it still throws a terminating error?

Copy link
Contributor Author

Copilot AI commented Dec 7, 2025

@copilot In the example shows how a state-changing command is using $PSCmdlet.ThrowTerminatingError(), is there a possible solution for where a user does not need to pass -ErrorAction 'Stop' when ...

No, there's no solution with $PSCmdlet.ThrowTerminatingError() that avoids requiring the caller to use -ErrorAction 'Stop' or set $ErrorActionPreference = 'Stop'. This is the inherent behavior of ThrowTerminatingError - it's command-terminating, not script-terminating.

Alternatives:

  1. Use Write-Error instead (recommended): Makes errors non-terminating by default, following standard PowerShell patterns. Callers control termination via -ErrorAction.

  2. Use throw: Creates truly terminating errors but produces poor error messages (documented as not allowed in public commands).

  3. Accept the current pattern: For state-changing commands where operation failures must terminate, requiring callers to use -ErrorAction 'Stop' or set $ErrorActionPreference = 'Stop' is acceptable and documented.

The recommendation is to use Write-Error for public commands unless the command is specifically designed to always fail on errors (like assert-style commands), in which case ThrowTerminatingError with documented caller requirements is acceptable.

@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot You say to use Write-Error for public commands unless the command is specifically designed to always fail on errors like assert-style commands. But let say we have this scenarion:

  1. We invoke Set-Property (without using ErrorAction parameter)
  2. Set-Property internally invokes Assert-ElevatedUser -ErrorAction 'Stop'
  3. Assert-ElevatedUser uses $PSCmdlet.ThrowTerminatingError() to throw terminating error that current user is not elevated
  4. Set-Property should stop executing and not continue after Assert-ElevatedUser

Does this scenario work as I expect it to?

Copy link
Contributor Author

Copilot AI commented Dec 7, 2025

@copilot You say to use Write-Error for public commands unless the command is specifically designed to always fail on errors like assert-style commands. But let say we have this scenarion:

  1. We invok...

NO, the scenario does NOT work as expected with just -ErrorAction 'Stop' on the Assert call.

When Assert-ElevatedUser uses $PSCmdlet.ThrowTerminatingError(), it creates a command-terminating error (stops Assert-ElevatedUser), but Set-Property will continue executing after the Assert call.

Solutions that DO work:

  1. Set $ErrorActionPreference = 'Stop' in Set-Property:

    function Set-Property {
        $ErrorActionPreference = 'Stop'
        Assert-ElevatedUser  # No -ErrorAction needed
        # Stops here if Assert fails
    }
  2. Use try-catch in Set-Property:

    function Set-Property {
        try {
            Assert-ElevatedUser -ErrorAction 'Stop'
            # Continues only if Assert succeeds
        } catch {
            # Handle or re-throw error
        }
    }
  3. Change Assert-ElevatedUser to use Write-Error instead (but this changes the assert semantics - it becomes non-terminating by default).

The -ErrorAction 'Stop' parameter on the Assert call alone is insufficient because ThrowTerminatingError doesn't propagate as a script-terminating error to the caller.

@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot But the line Set-Property: After Assert-ElevatedUser (should NOT show if assertion failed) was output even when it should have?

@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot Igonre if there would be breaking changes

Copy link
Contributor Author

Copilot AI commented Dec 7, 2025

@copilot Igonre if there would be breaking changes

Understood. Documentation updated in commit 586fd94 to remove assert-style commands from acceptable uses of $PSCmdlet.ThrowTerminatingError().

Changes:

  • Assert-style commands now recommended to use Write-Error (not ThrowTerminatingError)
  • Summary table updated to recommend Write-Error for assert-style commands
  • AI instructions updated accordingly

Rationale: With Write-Error, -ErrorAction 'Stop' alone works correctly for callers (standard PowerShell behavior), eliminating the complexity and non-intuitive requirements of ThrowTerminatingError.

@johlju

This comment was marked as duplicate.

1 similar comment
@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot I want you to verify that this scenarion actually works - stops execution with terminating error, take note where and how ErrorAction parameter is used. If it works see if we can simplify the documentation and AI instructions:

function Assert-ElevatedUser {
    [CmdletBinding()]
    param()
    
    # User is not elevated
    Write-Error -Message 'Not elevated' -Category PermissionDenied -ErrorId 'AEU001' -TargetObject 'CurrentUser' -ErrorAction 'Stop'
    return
    "ASSERT-AFTER -  WRONG - EXECUTION WRONGLY CONTINUED"
}

function Set-Property {
    [CmdletBinding()]
    param()
    
    Write-Host "Set-Property: Before Assert" -ForegroundColor Cyan
    Assert-ElevatedUser
    "SET-PROPERTY-AFTER - WRONG - EXECUTION WRONGLY CONTINUED"
}

# Start test
Set-Property

Remove AI instructions around error handling that is contradicting. Same for documentation, remove contradicting documentation to simplify.

Copy link
Contributor Author

Copilot AI commented Dec 7, 2025

@copilot I want you to verify that this scenarion actually works - stops execution with terminating error, take note where and how ErrorAction parameter is used. If it works see if we can simplify t...

✅ Verified - the scenario works perfectly! Execution stops with terminating error.

Key finding: Write-Error -ErrorAction 'Stop' inside the function creates a terminating error. The caller doesn't need to pass any -ErrorAction parameter.

Documentation simplified in commit b3ad133:

  • Removed all complex patterns and contradictions
  • Primary guidance: Use Write-Error with -ErrorAction 'Stop' for terminating errors
  • Removed all references to $PSCmdlet.ThrowTerminatingError() from public command guidance
  • Simplified summary table from 11 rows to 7 clear rows
  • AI instructions updated to match - simple and clear

The documentation now focuses on one simple pattern that works correctly.

@copilot I want you to verify that this scenarion actually works - stops execution with terminating error, take note where and how ErrorAction parameter is used. If it works see if we can simplify t...

✅ Verified - the scenario works perfectly! Execution stops with terminating error.

Key finding: Write-Error -ErrorAction 'Stop' inside the function creates a terminating error. The caller doesn't need to pass any -ErrorAction parameter.

Documentation simplified in commit b3ad133:

  • Removed all complex patterns and contradictions
  • Primary guidance: Use Write-Error with -ErrorAction 'Stop' for terminating errors
  • Removed all references to $PSCmdlet.ThrowTerminatingError() from public command guidance
  • Simplified summary table from 11 rows to 7 clear rows
  • AI instructions updated to match - simple and clear

The documentation now focuses on one simple pattern that works correctly.

Copilot AI changed the title CONTRIBUTING: Update error handling guidelines for public commands CONTRIBUTING: Simplify error handling guidelines - use Write-Error Dec 7, 2025
@johlju
Copy link
Member

johlju commented Dec 7, 2025

@copilot I think we can:

  1. Remove the section ##### .NET Method Exceptions, it feels like a duplicate wha we already have in the section ##### Exception Handling in Commands
  2. When using Write-Error together with -ErrorAction 'Stop' to throw a terminating error then there is no use of having return after it in those insances, right?

Copy link
Contributor Author

Copilot AI commented Dec 7, 2025

@copilot I think we can:

  1. Remove the section ##### .NET Method Exceptions, it feels like a duplicate wha we already have in the section ##### Exception Handling in Commands
    ...

Both changes made in commit 026f90f:

  1. ✅ Removed duplicate .NET Method Exceptions section - the pattern was already covered in Exception Handling in Commands
  2. ✅ Removed unnecessary return after Write-Error -ErrorAction 'Stop' - execution terminates immediately, so return is redundant

Also fixed duplicate closing backticks in the ValidateScript section.

@johlju johlju added the needs review The pull request needs a code review. label Dec 7, 2025
@johlju johlju marked this pull request as ready for review December 7, 2025 18:25
@johlju johlju requested a review from a team as a code owner December 7, 2025 18:25
@johlju
Copy link
Member

johlju commented Dec 7, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 7, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2ec8dc and 026f90f.

📒 Files selected for processing (3)
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md (1 hunks)
  • CHANGELOG.md (1 hunks)
  • CONTRIBUTING.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)

CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.md

Always update CHANGELOG.md Unreleased section

Files:

  • CHANGELOG.md

⚙️ CodeRabbit configuration file

CHANGELOG.md: # Changelog Guidelines

  • Always update the Unreleased section in CHANGELOG.md
  • Use Keep a Changelog format
  • Describe notable changes briefly, ≤2 items per change type
  • Reference issues using format issue #<issue_number>
  • No empty lines between list items in same section
  • Skip adding entry if same change already exists in Unreleased section
  • No duplicate sections or items in Unreleased section

Files:

  • CHANGELOG.md
**/*.md

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)

**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
Use '1.' for all items in ordered lists (1/1/1 numbering style)
Disable MD013 rule by adding a comment for tables/code blocks exceeding 80 characters
Empty lines required before/after code blocks and headings (except before line 1)
Escape backslashes in file paths only (not in code blocks)
Code blocks must specify language identifiers
Format parameters using bold in Markdown documentation
Format values and literals using inline code in Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths using inline code in Markdown documentation

Files:

  • CHANGELOG.md
  • CONTRIBUTING.md

⚙️ CodeRabbit configuration file

**/*.md: # Markdown Style Guidelines

  • Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
  • Use 2 spaces for indentation
  • Use '1.' for all items in ordered lists (1/1/1 numbering style)
  • Disable MD013 rule by adding a comment for tables/code blocks exceeding 80 characters
  • Empty lines required before/after code blocks and headings (except before line 1)
  • Escape backslashes in file paths only (not in code blocks)
  • Code blocks must specify language identifiers

Text Formatting

  • Parameters: bold
  • Values/literals: inline code
  • Resource/module/product names: italic
  • Commands/files/paths: inline code

Files:

  • CHANGELOG.md
  • CONTRIBUTING.md
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • Command: Public command
  • Function: Private function
  • Resource: DSC class-based resource

Build & Test Workflow Requirements

  • Run PowerShell script files from repository root
  • Setup build and test environment (once per pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File Organization

  • Public commands: source/Public/{CommandName}.ps1
  • Private functions: source/Private/{FunctionName}.ps1
  • Classes: source/Classes/{DependencyGroupNumber}.{ClassName}.ps1
  • Enums: source/Enum/{DependencyGroupNumber}.{EnumName}.ps1
  • Unit tests: tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1
  • Integration tests: tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Requirements

  • Follow instructions over existing code patterns
  • Follow PowerShell style and test guideline instructions strictly
  • Always update CHANGELOG.md Unreleased section
  • Localize all strings using string keys; remove any orphaned string keys
  • Check DscResource.Common before creating private functions
  • Separate reusable logic into private functions
  • DSC resources should always be created as class-based resources
  • Add unit tests for all commands/functions/resources
  • Add integration tests for all public commands and resources

Files:

  • CHANGELOG.md
  • CONTRIBUTING.md
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), and `-TargetObject` (object causing error)
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks, pass original exception to `Write-Error` using `-Exception` parameter and always use `return` after `Write-Error` to avoid further processing
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Follow PowerShell style and test guideline instructions strictly
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks; use `-ErrorAction SilentlyContinue` instead
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation without using try-catch-finally
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Information` for user-facing status updates, important operational messages, and non-error state changes
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use one space between type and variable (e.g., `[String] $name`)
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use PascalCase with Verb-Noun format for function names, using approved PowerShell verbs
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Warning` for non-fatal issues requiring attention, deprecated functionality usage, and configuration problems that don't block execution
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), and `-TargetObject` (object causing error)

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks; use `-ErrorAction SilentlyContinue` instead

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks, pass original exception to `Write-Error` using `-Exception` parameter and always use `return` after `Write-Error` to avoid further processing

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `New-*Exception` commands (such as `New-InvalidDataException`, `New-ArgumentException`, `New-InvalidOperationException`, `New-ObjectNotFoundException`, `New-InvalidResultException`, `New-NotImplementedException`) instead of `throw` for terminating errors in classes

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-11-14T08:43:03.398Z
Learnt from: raandree
Repo: dsccommunity/DscWorkshop PR: 193
File: build.yaml:34-42
Timestamp: 2025-11-14T08:43:03.398Z
Learning: In the dsccommunity/DscWorkshop repository, CHANGELOG.md is not currently maintained and should not be flagged as a required update.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-09-28T13:51:07.000Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/New-TerminatingError.ps1:33-36
Timestamp: 2025-09-28T13:51:07.000Z
Learning: In DhcpServerDsc repository, the user dan-hughes prefers to use error functions from DscResource.Common instead of creating custom error handling functions like New-TerminatingError.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidDataException` for invalid data errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation without using try-catch-finally

Applied to files:

  • CHANGELOG.md
  • CONTRIBUTING.md
  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-11-22T17:36:09.703Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must include comment-based help with .DESCRIPTION section containing Requirements and Known issues sections with a link to all open issues on GitHub

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Include examples in comment-based help for all parameter sets and combinations

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Unused mandatory parameters in `Set-TargetResource` and `Test-TargetResource` should include "Not used in <function_name>" in the help comment

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:57:47.617Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T17:57:47.617Z
Learning: SqlServerDsc-specific guidelines and requirements override general project guidelines and requirements

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidOperationException` for invalid operation errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ArgumentException` for argument-related errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ObjectNotFoundException` for object not found errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-09-06T19:12:11.414Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2189
File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130
Timestamp: 2025-09-06T19:12:11.414Z
Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.

Applied to files:

  • CONTRIBUTING.md
📚 Learning: 2025-11-27T18:00:35.078Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Follow PowerShell style and test guideline instructions strictly

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : For state-changing functions, use `SupportsShouldProcess` attribute with `ConfirmImpact` set to 'Low', 'Medium', or 'High' depending on risk

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Warning` for non-fatal issues requiring attention, deprecated functionality usage, and configuration problems that don't block execution

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Place ShouldProcess check immediately before each state-change operation

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid using `Write-Verbose` inside `$PSCmdlet.ShouldProcess`-block

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Host` cmdlet; use `Write-Verbose`, `Write-Information`, or other output streams instead

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Output` cmdlet; use `return` statement instead

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Verbose` for high-level execution flow and user-actionable information only

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Ensure ShouldProcess `$descriptionMessage` explains what will happen, `$confirmationMessage` succinctly asks for confirmation, and `$captionMessage` is short and descriptive without trailing period

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-09-06T19:18:18.857Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 0
File: :0-0
Timestamp: 2025-09-06T19:18:18.857Z
Learning: When a PowerShell command uses PSCmdlet.ShouldProcess and is called with -Verbose, the description message from ShouldProcess is automatically output as a verbose message. Therefore, there is no need to use Write-Verbose before or inside PSCmdlet.ShouldProcess blocks as PSCmdlet.ShouldProcess already provides the required verbose message through its description parameter.

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Information` for user-facing status updates, important operational messages, and non-error state changes

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-powershell.instructions.md
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md

511-511: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


511-511: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


512-512: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


512-512: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


513-513: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


513-513: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


514-514: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


514-514: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


515-515: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


515-515: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


516-516: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


516-516: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


517-517: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


517-517: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


518-518: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


518-518: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


519-519: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe

(MD055, table-pipe-style)


519-519: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: PSScriptAnalyzer
  • GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (3)
CHANGELOG.md (1)

62-66: Tighten wording and align with the finalized guidance

  • Include the explicit exceptions to the rule: throw is allowed in [ValidateScript()], and private functions may use $PSCmdlet.ThrowTerminatingError() when appropriate.
  • Rephrase the propagation note for clarity and add the verified pattern you endorse (Write-Error with -ErrorAction 'Stop' in callees of public commands).

Suggested tweak:

- 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
+ Updated CONTRIBUTING.md error handling guidelines: recommend `Write-Error`
+ for public commands (use `-ErrorAction 'Stop'` for terminating errors);
+ confirm that `$PSCmdlet.ThrowTerminatingError()` in callees does not propagate
+ as script‑terminating to callers unless they use `try/catch` or set
+ `$ErrorActionPreference = 'Stop'`. Document exceptions: throw may be used in
+ `[ValidateScript()]`; private functions may use `$PSCmdlet.ThrowTerminatingError()`
+ when appropriate

[ suggest_recommended_refactor ]

Please also verify the propagation statement with a minimal, reproducible snippet (copied from the PR discussion) and capture observed behavior (PowerShell 5.1 and 7.x). If needed, link the test results here for posterity.

CONTRIBUTING.md (1)

392-411: Remove the incorrect assertion and fix the ThrowTerminatingError behavior description. The statement that "the calling function continues executing after the error unless the caller uses $ErrorActionPreference = 'Stop' or try-catch" is incorrect. When $PSCmdlet.ThrowTerminatingError() is called, it stops execution of the calling function entirely—the Write-Output 'Continues' line will not execute. Revise the example to accurately reflect that terminating errors stop the caller by default, and remove the misleading "BUG: This executes!" comment. If distinguishing public command behavior is necessary, clarify that the restriction on $PSCmdlet.ThrowTerminatingError() in public commands is a style guideline preference, not a functional limitation.

Likely an incorrect or invalid review comment.

.github/instructions/dsc-community-style-guidelines-powershell.instructions.md (1)

94-104: Update public commands to align with new error handling guidance

Multiple public commands in source/Public/ still use $PSCmdlet.ThrowTerminatingError() despite the new style guide requiring Write-Error with -ErrorAction 'Stop' for terminating errors in public commands. Update the following files to use Write-Error instead:

  • Grant-SqlDscServerPermission.ps1
  • New-SqlDscRole.ps1
  • Import-SqlDscPreferredModule.ps1
  • Set-SqlDscDatabasePermission.ps1
  • New-SqlDscLogin.ps1
  • Set-SqlDscAgentOperator.ps1
  • Suspend-SqlDscDatabase.ps1
⛔ Skipped due to learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `New-*Exception` commands (such as `New-InvalidDataException`, `New-ArgumentException`, `New-InvalidOperationException`, `New-ObjectNotFoundException`, `New-InvalidResultException`, `New-NotImplementedException`) instead of `throw` for terminating errors in classes
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), and `-TargetObject` (object causing error)
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Applies to source/Public/*.ps1 : Public commands should be located in `source/Public/{CommandName}.ps1`
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks, pass original exception to `Write-Error` using `-Exception` parameter and always use `return` after `Write-Error` to avoid further processing
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation without using try-catch-finally
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ArgumentException` for argument-related errors instead of `throw` in MOF-based DSC resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks; use `-ErrorAction SilentlyContinue` instead
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2136
File: source/suffix.ps1:24-24
Timestamp: 2025-08-17T10:48:15.384Z
Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2189
File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130
Timestamp: 2025-09-06T19:12:11.414Z
Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Output` cmdlet; use `return` statement instead
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Warning` for non-fatal issues requiring attention, deprecated functionality usage, and configuration problems that don't block execution

Copilot AI requested a review from johlju December 7, 2025 18:43
@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94%. Comparing base (a2ec8dc) to head (92489fc).

Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff          @@
##           main   #2364   +/-   ##
====================================
  Coverage    94%     94%           
====================================
  Files       166     166           
  Lines      9808    9808           
====================================
  Hits       9249    9249           
  Misses      559     559           
Flag Coverage Δ
unit 94% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review The pull request needs a code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CONTRIBUTING: Update information around $PSCmdlet.ThrowTerminatingError

2 participants