Skip to content

Conversation

@hsurana06
Copy link
Member

@hsurana06 hsurana06 commented Sep 1, 2025

Description

Use -ErrorAction Stop for Az.ScVmm.internal\Get-AzScVmmMachine operations to fix expected exception being logged on console.
No change in API/Command.

The existing cmdlet works correctly, but the expected exception is printed to the console, which may confuse users into thinking the operation failed.

Before:
image

After:
image

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Copilot AI review requested due to automatic review settings September 1, 2025 18:08
@azure-client-tools-bot-prd
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves error handling in the Az.ScVmm module by adding -ErrorAction Stop to Get-AzScVmmMachine operations to prevent expected exceptions from being logged to the console. The changes focus on suppressing unhelpful error messages that confuse users, while maintaining the same functionality.

  • Added -ErrorAction Stop to all Get-AzScVmmMachine calls to properly handle expected exceptions
  • Enhanced error messages to include more context about the failure
  • Fixed minor documentation capitalization issues and typos

Reviewed Changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 7 comments.

File Description
src/ScVmm/ScVmm/ChangeLog.md Added changelog entry for exception suppression fix
src/ScVmm/ScVmm.Autorest/custom/*.ps1 Enhanced error handling with -ErrorAction Stop and improved error messages
src/ScVmm/ScVmm/help/*.md Fixed capitalization in synopsis and description text
src/ScVmm/ScVmm.Autorest/docs/*.md Fixed capitalization in synopsis and description text, corrected "MacAdders" typo

Comment on lines 227 to 228
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The variable check is comparing $vmObj but the error message still refers to "Virtual Machine $vmName". The logic checks if $vmObj is null but should check if the VM object retrieval failed, not if $machineObj is null as in the original code.

Suggested change
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
throw "Failed to fetch Virtual Machine Object using MachineId '$($machineObj.Id)' for Virtual Machine '$vmName'"

Copilot uses AI. Check for mistakes.
Comment on lines 186 to 187
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The variable check is comparing $vmObj but the error message still refers to "Virtual Machine $vmName". The logic checks if $vmObj is null but should check if the VM object retrieval failed, not if $machineObj is null as in the original code.

Suggested change
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
throw "Failed to fetch VM object for VM '$vmName' in Resource Group '$ResourceGroupName' (SubscriptionId '$SubscriptionId')"

Copilot uses AI. Check for mistakes.
Comment on lines 84 to 85
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The variable check is comparing $vmObj but the error message still refers to "Virtual Machine $vmName". The logic checks if $vmObj is null but should check if the VM object retrieval failed, not if $machineObj is null as in the original code.

Suggested change
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
throw "Failed to fetch Virtual Machine Object for MachineId $($machineObj.Id) (VM Name: $vmName)"

Copilot uses AI. Check for mistakes.
Comment on lines 276 to 277
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The variable check is comparing $vmObj but the error message still refers to "Virtual Machine $vmName". The logic checks if $vmObj is null but should check if the VM object retrieval failed, not if $machineObj is null as in the original code.

Suggested change
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
throw "Failed to fetch Virtual Machine Object for MachineId $($machineObj.Id) in Resource Group $ResourceGroupName (SubscriptionId $SubscriptionId)"

Copilot uses AI. Check for mistakes.
Comment on lines 267 to 268
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The variable check is comparing $vmObj but the error message still refers to "Virtual Machine $vmName". The logic checks if $vmObj is null but should check if the VM object retrieval failed, not if $machineObj is null as in the original code.

Suggested change
if ($null -eq $vmObj) {
throw "Failed to fetch Virtual Machine Object for Virtual Machine $vmName"
throw "Failed to fetch Virtual Machine Object for MachineId $($machineObj.Id) (VM Name: $vmName)"

Copilot uses AI. Check for mistakes.
@hsurana06
Copy link
Member Author

/azp run

@azure-pipelines
Copy link
Contributor

Commenter does not have sufficient privileges for PR 28490 in repo Azure/azure-powershell

@VeryEarly
Copy link
Collaborator

/azp run

@VeryEarly VeryEarly self-assigned this Sep 2, 2025
@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@VeryEarly
Copy link
Collaborator

please run

cd src/ScVmm/ScVmm.autorest
autorest
./build-module.ps1
cd src/ScVmm
git add
git commit -m "${commit-message}"
git push origin head

@isra-fel
Copy link
Member

isra-fel commented Sep 2, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@hsurana06
Copy link
Member Author

@VeryEarly, @isra-fel - Are there any additional steps required from me regarding the PR, and could you please share the timeline for when it will be rolled out to customers?

@VeryEarly
Copy link
Collaborator

@VeryEarly, @isra-fel - Are there any additional steps required from me regarding the PR, and could you please share the timeline for when it will be rolled out to customers?

please find timeline: https://github.com/Azure/azure-powershell/milestone/182

@VeryEarly VeryEarly merged commit 7c065fc into Azure:main Sep 4, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants