From 69921c108a893ee068011155084dd1c7c14071b9 Mon Sep 17 00:00:00 2001 From: Achyuth Maddala Sitaram <66455292+achyuth-ms@users.noreply.github.com> Date: Wed, 15 Apr 2026 13:39:35 -0700 Subject: [PATCH 1/4] error exception logging fix --- .../ErrorResponseExceptionHelperTests.cs | 190 ++++++++++++++++++ src/Sql/Sql/ChangeLog.md | 1 + src/Sql/Sql/Common/AzureSqlCmdletBase.cs | 40 ++-- .../Common/ErrorResponseExceptionHelper.cs | 96 +++++++++ .../AzureSqlManagedInstanceLinkAdapter.cs | 66 +----- src/Sql/Sql/Properties/AssemblyInfo.cs | 1 + .../Services/AzureSqlDeletedServerAdapter.cs | 41 ++-- 7 files changed, 337 insertions(+), 98 deletions(-) create mode 100644 src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs create mode 100644 src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs diff --git a/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs new file mode 100644 index 000000000000..936b8969d811 --- /dev/null +++ b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs @@ -0,0 +1,190 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +using Microsoft.Azure.Commands.Common.Exceptions; +using Microsoft.Azure.Commands.Sql.Common; +using Microsoft.Azure.Management.Sql.Models; +using Microsoft.Azure.ServiceManagement.Common.Models; +using Microsoft.Rest; +using Microsoft.WindowsAzure.Commands.ScenarioTest; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.Azure.Commands.Sql.Test.UnitTests +{ + public class ErrorResponseExceptionHelperTests + { + public ErrorResponseExceptionHelperTests(ITestOutputHelper output) + { + XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output)); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_WithBodyErrorMessage_ReturnsDetailedMessage() + { + // Arrange + var ex = new ErrorResponseException("Operation returned an invalid status code 'Forbidden'") + { + Body = new ErrorResponse(new ErrorDetail( + code: "RequestDisallowedByPolicy", + message: "Resource 'myserver' was disallowed by policy.")) + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.IsType(result); + Assert.Contains("Resource 'myserver' was disallowed by policy.", result.Message); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_WithBodyErrorDetails_IncludesDetailMessages() + { + // Arrange + var details = new List + { + new ErrorDetail(code: "PolicyViolation", message: "TLS version must be 1.2 or higher."), + new ErrorDetail(code: "PolicyViolation", message: "Public network access must be disabled.") + }; + var ex = new ErrorResponseException("Operation returned an invalid status code 'Forbidden'") + { + Body = new ErrorResponse(new ErrorDetail( + code: "RequestDisallowedByPolicy", + message: "Resource was disallowed by policy.", + details: details)) + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.IsType(result); + Assert.Contains("Resource was disallowed by policy.", result.Message); + Assert.Contains("TLS version must be 1.2 or higher.", result.Message); + Assert.Contains("Public network access must be disabled.", result.Message); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_WithResponseContentArmFormat_ParsesErrorMessage() + { + // Arrange — Body is null, but Response.Content has the ARM error JSON + var httpResponse = new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new StringContent("{\"error\":{\"code\":\"ResourceNotFound\",\"message\":\"The Resource 'Microsoft.Sql/servers/myserver' was not found.\"}}") + }; + var ex = new ErrorResponseException("Operation returned an invalid status code 'NotFound'") + { + Response = new HttpResponseMessageWrapper(httpResponse, "{\"error\":{\"code\":\"ResourceNotFound\",\"message\":\"The Resource 'Microsoft.Sql/servers/myserver' was not found.\"}}") + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.IsType(result); + Assert.Contains("The Resource 'Microsoft.Sql/servers/myserver' was not found.", result.Message); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_WithResponseContentFlatFormat_ParsesMessage() + { + // Arrange — Body is null, Response.Content has flat "Message" key + var httpResponse = new HttpResponseMessage(HttpStatusCode.BadRequest) + { + Content = new StringContent("{\"Message\":\"Only one active directory allowed.\"}") + }; + var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") + { + Response = new HttpResponseMessageWrapper(httpResponse, "{\"Message\":\"Only one active directory allowed.\"}") + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.IsType(result); + Assert.Contains("Only one active directory allowed.", result.Message); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_WithNoBodyAndNoContent_ReturnsOriginalMessage() + { + // Arrange — No body, no response content + var ex = new ErrorResponseException("Operation returned an invalid status code 'InternalServerError'"); + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.IsType(result); + Assert.Equal("Operation returned an invalid status code 'InternalServerError'", result.Message); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_WithInvalidJsonContent_ReturnsOriginalMessage() + { + // Arrange — Response content is not valid JSON + var httpResponse = new HttpResponseMessage(HttpStatusCode.BadRequest) + { + Content = new StringContent("This is not JSON") + }; + var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") + { + Response = new HttpResponseMessageWrapper(httpResponse, "This is not JSON") + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.IsType(result); + Assert.Equal("Operation returned an invalid status code 'BadRequest'", result.Message); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_BodyTakesPrecedenceOverResponseContent() + { + // Arrange — Both Body and Response.Content have error info; Body should win + var httpResponse = new HttpResponseMessage(HttpStatusCode.Forbidden) + { + Content = new StringContent("{\"error\":{\"code\":\"Forbidden\",\"message\":\"Response content message\"}}") + }; + var ex = new ErrorResponseException("Operation returned an invalid status code 'Forbidden'") + { + Body = new ErrorResponse(new ErrorDetail( + code: "RequestDisallowedByPolicy", + message: "Body error message")), + Response = new HttpResponseMessageWrapper(httpResponse, "{\"error\":{\"code\":\"Forbidden\",\"message\":\"Response content message\"}}") + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert + Assert.Contains("Body error message", result.Message); + Assert.DoesNotContain("Response content message", result.Message); + } + } +} diff --git a/src/Sql/Sql/ChangeLog.md b/src/Sql/Sql/ChangeLog.md index cce53e9621b6..5fda096395ac 100644 --- a/src/Sql/Sql/ChangeLog.md +++ b/src/Sql/Sql/ChangeLog.md @@ -18,6 +18,7 @@ - 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. ## Version 6.4.1 * Add support for the versionless AKV keys. diff --git a/src/Sql/Sql/Common/AzureSqlCmdletBase.cs b/src/Sql/Sql/Common/AzureSqlCmdletBase.cs index 9fd407faef14..080107700c79 100644 --- a/src/Sql/Sql/Common/AzureSqlCmdletBase.cs +++ b/src/Sql/Sql/Common/AzureSqlCmdletBase.cs @@ -20,6 +20,7 @@ using System.Management.Automation; using Microsoft.Azure.Commands.Common.Authentication.Abstractions; using Microsoft.Azure.Commands.ResourceManager.Common.ArgumentCompleters; +using Microsoft.Azure.Management.Sql.Models; namespace Microsoft.Azure.Commands.Sql.Common { @@ -118,29 +119,36 @@ protected virtual string GetConfirmActionProcessMessage() /// public override void ExecuteCmdlet() { - ModelAdapter = InitModelAdapter(); - M model = GetEntity(); - M updatedModel = ApplyUserInputToModel(model); - M responseModel = default(M); - ConfirmAction(GetConfirmActionProcessMessage(), GetResourceId(updatedModel), () => + try { - responseModel = PersistChanges(updatedModel); - }); + ModelAdapter = InitModelAdapter(); + M model = GetEntity(); + M updatedModel = ApplyUserInputToModel(model); + M responseModel = default(M); + ConfirmAction(GetConfirmActionProcessMessage(), GetResourceId(updatedModel), () => + { + responseModel = PersistChanges(updatedModel); + }); - if (responseModel != null) - { - if (WriteResult()) + if (responseModel != null) { - WriteObject(TransformModelToOutputObject(responseModel), true); + if (WriteResult()) + { + WriteObject(TransformModelToOutputObject(responseModel), true); + } } - } - else - { - if (WriteResult()) + else { - WriteObject(TransformModelToOutputObject(updatedModel)); + if (WriteResult()) + { + WriteObject(TransformModelToOutputObject(updatedModel)); + } } } + catch (ErrorResponseException ex) + { + throw ErrorResponseExceptionHelper.CreateFrom(ex); + } } } } diff --git a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs new file mode 100644 index 000000000000..d3b6d2b2d6c6 --- /dev/null +++ b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs @@ -0,0 +1,96 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +using Microsoft.Azure.Commands.Common.Exceptions; +using Microsoft.Azure.Management.Sql.Models; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; + +namespace Microsoft.Azure.Commands.Sql.Common +{ + /// + /// Helper class to convert ErrorResponseException to AzPSCloudException with descriptive error messages. + /// Due to a change in the SDK generator when common-types v5 ErrorResponse schema is used, + /// the ErrorResponseException.Message is not populated with the actual error details. + /// This helper extracts the real error message from the response body. + /// + internal static class ErrorResponseExceptionHelper + { + /// + /// Creates an AzPSCloudException from an ErrorResponseException by extracting + /// the actual error message from the response body. + /// + /// The original ErrorResponseException + /// An AzPSCloudException with the descriptive error message + internal static AzPSCloudException CreateFrom(ErrorResponseException ex) + { + // First try to get the message from the structured Body object + string detailedMessage = ex.Body?.Error?.Message; + + // Append error details if available (e.g., Azure Policy violation details) + if (!string.IsNullOrEmpty(detailedMessage) && ex.Body?.Error?.Details != null && ex.Body.Error.Details.Any()) + { + var sb = new StringBuilder(detailedMessage); + foreach (var detail in ex.Body.Error.Details) + { + if (!string.IsNullOrEmpty(detail.Message)) + { + sb.AppendLine(); + sb.Append(detail.Message); + } + } + detailedMessage = sb.ToString(); + } + + // If that didn't work, try parsing the raw response content + if (string.IsNullOrEmpty(detailedMessage) && ex.Response != null && !string.IsNullOrEmpty(ex.Response.Content)) + { + try + { + var content = JsonConvert.DeserializeObject>(ex.Response.Content); + + if (content.ContainsKey("error")) + { + JObject errorResponse = (JObject)content["error"]; + JToken errorMessage; + if (errorResponse.TryGetValue("message", StringComparison.InvariantCultureIgnoreCase, out errorMessage)) + { + detailedMessage = errorMessage.ToString(); + } + } + else if (content.ContainsKey("Message")) + { + detailedMessage = content["Message"].ToString(); + } + } + catch (JsonException) + { + // JSON parsing failed — fall through to use original message + } + } + + if (!string.IsNullOrEmpty(detailedMessage)) + { + return new AzPSCloudException(detailedMessage); + } + + // Could not extract a better message — wrap original as-is + return new AzPSCloudException(ex.Message); + } + } +} diff --git a/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs b/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs index 6675e718f585..abbebf0794f8 100644 --- a/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs +++ b/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs @@ -54,15 +54,8 @@ public AzureSqlManagedInstanceLinkAdapter(IAzureContext context) /// The managed instance link public AzureSqlManagedInstanceLinkModel GetManagedInstanceLink(string resourceGroupName, string instanceName, string distributedAvailabilityGroupName) { - try - { - var resp = Communicator.Get(resourceGroupName, instanceName, distributedAvailabilityGroupName); - return CreateManagedInstanceLinkModelFromResponse(resourceGroupName, instanceName, resp); - } - catch (ErrorResponseException ex) - { - throw CreateExceptionWithDescriptiveErrorMessage(ex); - } + var resp = Communicator.Get(resourceGroupName, instanceName, distributedAvailabilityGroupName); + return CreateManagedInstanceLinkModelFromResponse(resourceGroupName, instanceName, resp); } /// @@ -73,15 +66,8 @@ public AzureSqlManagedInstanceLinkModel GetManagedInstanceLink(string resourceGr /// A list of all the server trust certificates public List ListManagedInstanceLinks(string resourceGroupName, string instanceName) { - try - { - var resp = Communicator.List(resourceGroupName, instanceName); - return resp.Select((dag) => CreateManagedInstanceLinkModelFromResponse(resourceGroupName, instanceName, dag)).ToList(); - } - catch (ErrorResponseException ex) - { - throw CreateExceptionWithDescriptiveErrorMessage(ex); - } + var resp = Communicator.List(resourceGroupName, instanceName); + return resp.Select((dag) => CreateManagedInstanceLinkModelFromResponse(resourceGroupName, instanceName, dag)).ToList(); } /// @@ -130,7 +116,7 @@ internal AzureSqlManagedInstanceLinkModel CreateManagedInstanceLink(AzureSqlMana throw new ErrorResponseException("Allowed values for instance link role are 'Primary' or 'Secondary'."); } - throw CreateExceptionWithDescriptiveErrorMessage(ex); + throw; } } @@ -141,19 +127,12 @@ internal AzureSqlManagedInstanceLinkModel CreateManagedInstanceLink(AzureSqlMana /// The updated Azure Sql Managed Instance Link internal AzureSqlManagedInstanceLinkModel UpdateManagedInstanceLink(AzureSqlManagedInstanceLinkModel model) { - try + var resp = Communicator.Update(model.ResourceGroupName, model.InstanceName, model.Name, new Management.Sql.Models.DistributedAvailabilityGroup { - var resp = Communicator.Update(model.ResourceGroupName, model.InstanceName, model.Name, new Management.Sql.Models.DistributedAvailabilityGroup - { - ReplicationMode = model.ReplicationMode, - }); + ReplicationMode = model.ReplicationMode, + }); - return CreateManagedInstanceLinkModelFromResponse(model.ResourceGroupName, model.InstanceName, resp); - } - catch (ErrorResponseException ex) - { - throw CreateExceptionWithDescriptiveErrorMessage(ex); - } + return CreateManagedInstanceLinkModelFromResponse(model.ResourceGroupName, model.InstanceName, resp); } /// @@ -164,14 +143,7 @@ internal AzureSqlManagedInstanceLinkModel UpdateManagedInstanceLink(AzureSqlMana /// Name of the instance link to delete public void RemoveManagedInstanceLink(string resourceGroupName, string instanceName, string managedInstanceLinkName) { - try - { - Communicator.Remove(resourceGroupName, instanceName, managedInstanceLinkName); - } - catch (ErrorResponseException ex) - { - throw CreateExceptionWithDescriptiveErrorMessage(ex); - } + Communicator.Remove(resourceGroupName, instanceName, managedInstanceLinkName); } /// @@ -199,7 +171,7 @@ internal AzureSqlManagedInstanceLinkModel FailoverManagedInstanceLink(AzureSqlMa throw new ErrorResponseException("Allowed values for failover type are 'Planned' or 'ForcedAllowDataLoss'."); } - throw CreateExceptionWithDescriptiveErrorMessage(ex); + throw; } } @@ -235,21 +207,5 @@ private static AzureSqlManagedInstanceLinkModel CreateManagedInstanceLinkModelFr return managedInstanceLinkModel; } - /// - /// Due to some change in SDK generator, proper messages from REST aren't propagated as a exception message. - /// For this reason the error message of the PS cmdlet will always be generic that is 'Operation returned an invalid status code '{0}''. - /// In order to get the correct error message we need to extract it from Body of the original exception. - /// - /// The new ErrorResponseException that will have non-generic error message. - private ErrorResponseException CreateExceptionWithDescriptiveErrorMessage(ErrorResponseException originalException) - { - ErrorResponseException responseException = new ErrorResponseException(originalException.Body.Error.Message, originalException.InnerException); - responseException.Body = originalException.Body; - responseException.Request = originalException.Request; - responseException.Response = originalException.Response; - - return responseException; - } - } } diff --git a/src/Sql/Sql/Properties/AssemblyInfo.cs b/src/Sql/Sql/Properties/AssemblyInfo.cs index 0258f0f989e3..dfb2fc6bad1f 100644 --- a/src/Sql/Sql/Properties/AssemblyInfo.cs +++ b/src/Sql/Sql/Properties/AssemblyInfo.cs @@ -48,4 +48,5 @@ [assembly: AssemblyFileVersion("6.4.1")] #if !SIGN [assembly: InternalsVisibleTo("Microsoft.Azure.PowerShell.Cmdlets.Resources.Test")] +[assembly: InternalsVisibleTo("Microsoft.Azure.PowerShell.Cmdlets.Sql.Test")] #endif diff --git a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs index 8c2b6d2b4f45..54de5f4081ef 100644 --- a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs +++ b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs @@ -14,6 +14,7 @@ using Microsoft.Azure.Commands.Common.Authentication.Abstractions; using Microsoft.Azure.Commands.Sql; +using Microsoft.Azure.Commands.Sql.Common; using Microsoft.Azure.Commands.Sql.Server.Model; using Microsoft.Azure.Management.Sql.Models; using System; @@ -61,7 +62,19 @@ public DeletedServer GetDeletedServer(string location, string serverName, string } catch (ErrorResponseException ex) { - throw GetImprovedErrorResponseException(ex, location, serverName); + if (ex.Response?.StatusCode == System.Net.HttpStatusCode.NotFound) + { + throw new ErrorResponseException( + string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location), + ex.InnerException) + { + Body = ex.Body, + Request = ex.Request, + Response = ex.Response + }; + } + + throw ErrorResponseExceptionHelper.CreateFrom(ex); } } @@ -110,31 +123,5 @@ public AzureSqlDeletedServerModel CreateDeletedServerModelFromResponse(DeletedSe return model; } - /// - /// Creates an improved ErrorResponseException with a more descriptive error message for NotFound scenarios. - /// - /// The original exception from the SDK - /// The location where the server was being searched - /// The name of the server being searched for - /// An ErrorResponseException with an improved error message - private ErrorResponseException GetImprovedErrorResponseException(ErrorResponseException originalException, string location, string serverName) - { - string improvedMessage = originalException.Body?.Error?.Message; - - // If it's a NotFound error, provide a more helpful message - if (originalException.Response?.StatusCode == System.Net.HttpStatusCode.NotFound) - { - improvedMessage = string.Format( - Properties.Resources.DeletedServerNotFoundInLocation, - serverName, location); - } - - ErrorResponseException improvedException = new ErrorResponseException(improvedMessage, originalException.InnerException); - improvedException.Body = originalException.Body; - improvedException.Request = originalException.Request; - improvedException.Response = originalException.Response; - - return improvedException; - } } } \ No newline at end of file From 4558da37c3bd95dc301ca265351a76ee4db2a516 Mon Sep 17 00:00:00 2001 From: Achyuth Maddala Sitaram <66455292+achyuth-ms@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:43:42 -0700 Subject: [PATCH 2/4] fix review comments --- src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs | 9 ++------- .../Services/AzureSqlManagedInstanceLinkAdapter.cs | 4 ++-- .../Server/Services/AzureSqlDeletedServerAdapter.cs | 11 ++++------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs index d3b6d2b2d6c6..46b1a31e4826 100644 --- a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs +++ b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs @@ -84,13 +84,8 @@ internal static AzPSCloudException CreateFrom(ErrorResponseException ex) } } - if (!string.IsNullOrEmpty(detailedMessage)) - { - return new AzPSCloudException(detailedMessage); - } - - // Could not extract a better message — wrap original as-is - return new AzPSCloudException(ex.Message); + var message = !string.IsNullOrEmpty(detailedMessage) ? detailedMessage : ex.Message; + return new AzPSCloudException(message, message, ex); } } } diff --git a/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs b/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs index abbebf0794f8..964e99f3aee0 100644 --- a/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs +++ b/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs @@ -111,7 +111,7 @@ internal AzureSqlManagedInstanceLinkModel CreateManagedInstanceLink(AzureSqlMana if (ex.Response.Content.Contains("instanceLinkRole") && ex.Response.StatusCode == System.Net.HttpStatusCode.BadRequest && - (!model.FailoverMode.Equals("Primary") && !model.FailoverMode.Equals("Secondary"))) + (!model.InstanceLinkRole.Equals("Primary") && !model.InstanceLinkRole.Equals("Secondary"))) { throw new ErrorResponseException("Allowed values for instance link role are 'Primary' or 'Secondary'."); } @@ -166,7 +166,7 @@ internal AzureSqlManagedInstanceLinkModel FailoverManagedInstanceLink(AzureSqlMa { if (ex.Response.Content.Contains("failoverType") && ex.Response.StatusCode == System.Net.HttpStatusCode.BadRequest && - (!model.FailoverMode.Equals("Planned") || !model.FailoverMode.Equals("ForcedAllowDataLoss"))) + (!model.FailoverMode.Equals("Planned") && !model.FailoverMode.Equals("ForcedAllowDataLoss"))) { throw new ErrorResponseException("Allowed values for failover type are 'Planned' or 'ForcedAllowDataLoss'."); } diff --git a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs index 54de5f4081ef..f6baacd0d03d 100644 --- a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs +++ b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs @@ -13,6 +13,7 @@ // ---------------------------------------------------------------------------------- using Microsoft.Azure.Commands.Common.Authentication.Abstractions; +using Microsoft.Azure.Commands.Common.Exceptions; using Microsoft.Azure.Commands.Sql; using Microsoft.Azure.Commands.Sql.Common; using Microsoft.Azure.Commands.Sql.Server.Model; @@ -64,14 +65,10 @@ public DeletedServer GetDeletedServer(string location, string serverName, string { if (ex.Response?.StatusCode == System.Net.HttpStatusCode.NotFound) { - throw new ErrorResponseException( + throw new AzPSResourceNotFoundCloudException( string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location), - ex.InnerException) - { - Body = ex.Body, - Request = ex.Request, - Response = ex.Response - }; + string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location), + ex); } throw ErrorResponseExceptionHelper.CreateFrom(ex); From fee64a0b8975824358ae2ad291b4e985d6451804 Mon Sep 17 00:00:00 2001 From: Achyuth Maddala Sitaram <66455292+achyuth-ms@users.noreply.github.com> Date: Fri, 17 Apr 2026 10:41:53 -0700 Subject: [PATCH 3/4] fix pipeline and address comments --- .../ErrorResponseExceptionHelperTests.cs | 27 +++++++++++++ .../Common/ErrorResponseExceptionHelper.cs | 39 +++++++++++++------ .../AzureSqlManagedInstanceLinkAdapter.cs | 8 ++-- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs index 936b8969d811..df65779842b1 100644 --- a/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs +++ b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs @@ -186,5 +186,32 @@ public void CreateFrom_BodyTakesPrecedenceOverResponseContent() Assert.Contains("Body error message", result.Message); Assert.DoesNotContain("Response content message", result.Message); } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void CreateFrom_PreservesInnerExceptionAndContext() + { + // Arrange + var httpResponse = new HttpResponseMessage(HttpStatusCode.Forbidden); + var ex = new ErrorResponseException("Operation returned an invalid status code 'Forbidden'") + { + Body = new ErrorResponse(new ErrorDetail( + code: "AuthorizationFailed", + message: "Authorization failed.")), + Request = new HttpRequestMessageWrapper(new HttpRequestMessage(HttpMethod.Get, "https://management.azure.com/test"), ""), + Response = new HttpResponseMessageWrapper(httpResponse, "") + }; + + // Act + var result = ErrorResponseExceptionHelper.CreateFrom(ex); + + // Assert — inner exception is preserved + Assert.IsType(result.InnerException); + // Assert — ErrorCode is propagated + Assert.Equal("AuthorizationFailed", result.Data["ErrorCode"]); + // Assert — Request and Response are in Data + Assert.NotNull(result.Data["Request"]); + Assert.NotNull(result.Data["Response"]); + } } } diff --git a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs index 46b1a31e4826..2c1b9c8a17dc 100644 --- a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs +++ b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs @@ -14,10 +14,8 @@ using Microsoft.Azure.Commands.Common.Exceptions; using Microsoft.Azure.Management.Sql.Models; -using Newtonsoft.Json; using Newtonsoft.Json.Linq; using System; -using System.Collections.Generic; using System.Linq; using System.Text; @@ -62,30 +60,49 @@ internal static AzPSCloudException CreateFrom(ErrorResponseException ex) { try { - var content = JsonConvert.DeserializeObject>(ex.Response.Content); + var parsed = JObject.Parse(ex.Response.Content); - if (content.ContainsKey("error")) + var errorObj = parsed["error"] as JObject; + if (errorObj != null) { - JObject errorResponse = (JObject)content["error"]; JToken errorMessage; - if (errorResponse.TryGetValue("message", StringComparison.InvariantCultureIgnoreCase, out errorMessage)) + if (errorObj.TryGetValue("message", StringComparison.InvariantCultureIgnoreCase, out errorMessage)) { detailedMessage = errorMessage.ToString(); } } - else if (content.ContainsKey("Message")) + else { - detailedMessage = content["Message"].ToString(); + var messageToken = parsed["Message"]; + if (messageToken != null) + { + detailedMessage = messageToken.ToString(); + } } } - catch (JsonException) + catch (Exception) { - // JSON parsing failed — fall through to use original message + // JSON parsing or property access failed — fall through to use original message } } var message = !string.IsNullOrEmpty(detailedMessage) ? detailedMessage : ex.Message; - return new AzPSCloudException(message, message, ex); + var wrappedException = new AzPSCloudException(message, message, ex); + + if (ex.Request != null) + { + wrappedException.Data["Request"] = ex.Request; + } + if (ex.Response != null) + { + wrappedException.Data["Response"] = ex.Response; + } + if (!string.IsNullOrEmpty(ex.Body?.Error?.Code)) + { + wrappedException.Data["ErrorCode"] = ex.Body.Error.Code; + } + + return wrappedException; } } } diff --git a/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs b/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs index 964e99f3aee0..ea2abace5750 100644 --- a/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs +++ b/src/Sql/Sql/DistributedAvailabilityGroup/Services/AzureSqlManagedInstanceLinkAdapter.cs @@ -13,6 +13,7 @@ // ---------------------------------------------------------------------------------- using Microsoft.Azure.Commands.Common.Authentication.Abstractions; +using Microsoft.Azure.Commands.Sql.Common; using Microsoft.Azure.Commands.Sql.ManagedInstanceHybridLink.Model; using Microsoft.Azure.Management.Sql.Models; using System.Collections.Generic; @@ -111,12 +112,13 @@ internal AzureSqlManagedInstanceLinkModel CreateManagedInstanceLink(AzureSqlMana if (ex.Response.Content.Contains("instanceLinkRole") && ex.Response.StatusCode == System.Net.HttpStatusCode.BadRequest && - (!model.InstanceLinkRole.Equals("Primary") && !model.InstanceLinkRole.Equals("Secondary"))) + (!string.Equals(model.InstanceLinkRole, "Primary", System.StringComparison.OrdinalIgnoreCase) && + !string.Equals(model.InstanceLinkRole, "Secondary", System.StringComparison.OrdinalIgnoreCase))) { throw new ErrorResponseException("Allowed values for instance link role are 'Primary' or 'Secondary'."); } - throw; + throw ErrorResponseExceptionHelper.CreateFrom(ex); } } @@ -171,7 +173,7 @@ internal AzureSqlManagedInstanceLinkModel FailoverManagedInstanceLink(AzureSqlMa throw new ErrorResponseException("Allowed values for failover type are 'Planned' or 'ForcedAllowDataLoss'."); } - throw; + throw ErrorResponseExceptionHelper.CreateFrom(ex); } } From 31f14f79e9d4b3b2f1671eaaed5b6e72854db357 Mon Sep 17 00:00:00 2001 From: Achyuth Maddala Sitaram <66455292+achyuth-ms@users.noreply.github.com> Date: Fri, 17 Apr 2026 15:29:01 -0700 Subject: [PATCH 4/4] address comments --- .../ErrorResponseExceptionHelperTests.cs | 8 ++++---- src/Sql/Sql/ChangeLog.md | 2 +- .../Sql/Common/ErrorResponseExceptionHelper.cs | 18 +++++++----------- .../Services/AzureSqlDeletedServerAdapter.cs | 5 +++-- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs index df65779842b1..e8c47f3852cd 100644 --- a/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs +++ b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs @@ -208,10 +208,10 @@ public void CreateFrom_PreservesInnerExceptionAndContext() // Assert — inner exception is preserved Assert.IsType(result.InnerException); // Assert — ErrorCode is propagated - Assert.Equal("AuthorizationFailed", result.Data["ErrorCode"]); - // Assert — Request and Response are in Data - Assert.NotNull(result.Data["Request"]); - Assert.NotNull(result.Data["Response"]); + Assert.True(result.Data.Contains("CloudErrorCode")); + // Assert — Request and Response are set + Assert.NotNull(result.Request); + Assert.NotNull(result.Response); } } } diff --git a/src/Sql/Sql/ChangeLog.md b/src/Sql/Sql/ChangeLog.md index 5fda096395ac..c01ab0a8ce1c 100644 --- a/src/Sql/Sql/ChangeLog.md +++ b/src/Sql/Sql/ChangeLog.md @@ -18,7 +18,7 @@ - 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. +* Fixed error handling in Az.Sql cmdlets that inherit from `AzureSqlCmdletBase` 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. ## Version 6.4.1 * Add support for the versionless AKV keys. diff --git a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs index 2c1b9c8a17dc..d8dfd384b2fa 100644 --- a/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs +++ b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs @@ -66,7 +66,7 @@ internal static AzPSCloudException CreateFrom(ErrorResponseException ex) if (errorObj != null) { JToken errorMessage; - if (errorObj.TryGetValue("message", StringComparison.InvariantCultureIgnoreCase, out errorMessage)) + if (errorObj.TryGetValue("message", StringComparison.OrdinalIgnoreCase, out errorMessage)) { detailedMessage = errorMessage.ToString(); } @@ -87,19 +87,15 @@ internal static AzPSCloudException CreateFrom(ErrorResponseException ex) } var message = !string.IsNullOrEmpty(detailedMessage) ? detailedMessage : ex.Message; - var wrappedException = new AzPSCloudException(message, message, ex); - - if (ex.Request != null) - { - wrappedException.Data["Request"] = ex.Request; - } - if (ex.Response != null) + var wrappedException = new AzPSCloudException(message, message, ex) { - wrappedException.Data["Response"] = ex.Response; - } + Request = ex.Request, + Response = ex.Response, + }; + if (!string.IsNullOrEmpty(ex.Body?.Error?.Code)) { - wrappedException.Data["ErrorCode"] = ex.Body.Error.Code; + wrappedException.Data["CloudErrorCode"] = ex.Body.Error.Code; } return wrappedException; diff --git a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs index f6baacd0d03d..e3ec63caedf4 100644 --- a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs +++ b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs @@ -65,9 +65,10 @@ public DeletedServer GetDeletedServer(string location, string serverName, string { if (ex.Response?.StatusCode == System.Net.HttpStatusCode.NotFound) { + var notFoundMessage = string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location); throw new AzPSResourceNotFoundCloudException( - string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location), - string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location), + notFoundMessage, + notFoundMessage, ex); }