[Sql] Fix ErrorResponseException handling to surface descriptive error messages across all Az.Sql cmdlets#29410
Open
achyuth-ms wants to merge 4 commits intoAzure:mainfrom
Open
[Sql] Fix ErrorResponseException handling to surface descriptive error messages across all Az.Sql cmdlets#29410achyuth-ms wants to merge 4 commits intoAzure:mainfrom
achyuth-ms wants to merge 4 commits intoAzure:mainfrom
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
Contributor
|
Azure Pipelines: 4 pipeline(s) require an authorized user to comment /azp run to run. |
Member
Author
|
/azp run |
Contributor
|
Commenter does not have sufficient privileges for PR 29410 in repo Azure/azure-powershell |
Contributor
|
Azure Pipelines: 4 pipeline(s) require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve Az.Sql user experience by ensuring ErrorResponseException failures surface descriptive REST error messages (for example, Azure Policy violation details) instead of generic SDK exception text, primarily by centralizing exception rewriting in the Sql cmdlet base.
Changes:
- Added
ErrorResponseExceptionHelperand updatedAzureSqlCmdletBase.ExecuteCmdlet()to wrapErrorResponseExceptioninto anAzPSCloudExceptionwith a more descriptive message. - Refactored a few adapters (deleted server, managed instance link) to rely on the centralized error handling and removed local “message improvement” helpers.
- Added unit tests for the new helper and updated Sql
ChangeLog.mdandInternalsVisibleTofor the new test assembly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs | New helper to extract detailed error messages from ErrorResponseException body/response content. |
| src/Sql/Sql/Common/AzureSqlCmdletBase.cs | Wraps ErrorResponseException in a centralized catch to ensure descriptive messages for cmdlets using this base. |
| src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs | Special-cases NotFound for deleted server lookup; otherwise uses the new helper. |
| src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs | Removes local exception-rewrite method and rethrows to allow centralized handling. |
| src/Sql/Sql/Properties/AssemblyInfo.cs | Adds InternalsVisibleTo for the Sql test assembly. |
| src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs | New unit tests validating message extraction precedence and parsing behavior. |
| src/Sql/Sql/ChangeLog.md | Adds an Upcoming Release note describing the improved error handling behavior. |
ericshape
approved these changes
Apr 16, 2026
Member
|
/azp run |
Contributor
|
Commenter does not have sufficient privileges for PR 29410 in repo Azure/azure-powershell |
Collaborator
|
/azp run |
Contributor
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
Author
|
/azp run |
Contributor
|
Commenter does not have sufficient privileges for PR 29410 in repo Azure/azure-powershell |
| - Additional information about change #1 | ||
| --> | ||
| ## Upcoming Release | ||
| * Fixed error handling across all Az.Sql cmdlets to surface descriptive error messages instead of generic 'Operation returned an invalid status code' when API calls fail. This restores meaningful error details such as Azure Policy violation messages. |
There was a problem hiding this comment.
The changelog entry claims the fix applies "across all Az.Sql cmdlets", but there are Az.Sql cmdlets that do not go through AzureSqlCmdletBase (e.g., some inherit directly from AzureRMCmdlet). Consider rewording this entry to accurately scope the change to cmdlets impacted by ErrorResponseException message propagation.
Comment on lines
+93
to
+97
| { | ||
| wrappedException.Data["Request"] = ex.Request; | ||
| } | ||
| if (ex.Response != null) | ||
| { |
There was a problem hiding this comment.
ErrorResponseExceptionHelper is storing request/response objects only in Exception.Data. For consistency with other cmdlets that wrap into AzPSCloudException (and to ensure standard formatting/telemetry continues to work), set the AzPSCloudException.Request and .Response properties directly instead of (or in addition to) custom Data keys like "Request"/"Response".
Suggested change
| { | |
| wrappedException.Data["Request"] = ex.Request; | |
| } | |
| if (ex.Response != null) | |
| { | |
| { | |
| wrappedException.Request = ex.Request; | |
| wrappedException.Data["Request"] = ex.Request; | |
| } | |
| if (ex.Response != null) | |
| { | |
| wrappedException.Response = ex.Response; |
Collaborator
|
/azp run |
Contributor
|
Azure Pipelines successfully started running 3 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.