diff --git a/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs new file mode 100644 index 000000000000..e8c47f3852cd --- /dev/null +++ b/src/Sql/Sql.Test/UnitTests/ErrorResponseExceptionHelperTests.cs @@ -0,0 +1,217 @@ +// ---------------------------------------------------------------------------------- +// +// 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); + } + + [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.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 cce53e9621b6..c01ab0a8ce1c 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 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/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..d8dfd384b2fa --- /dev/null +++ b/src/Sql/Sql/Common/ErrorResponseExceptionHelper.cs @@ -0,0 +1,104 @@ +// ---------------------------------------------------------------------------------- +// +// 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.Linq; +using System; +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 parsed = JObject.Parse(ex.Response.Content); + + var errorObj = parsed["error"] as JObject; + if (errorObj != null) + { + JToken errorMessage; + if (errorObj.TryGetValue("message", StringComparison.OrdinalIgnoreCase, out errorMessage)) + { + detailedMessage = errorMessage.ToString(); + } + } + else + { + var messageToken = parsed["Message"]; + if (messageToken != null) + { + detailedMessage = messageToken.ToString(); + } + } + } + catch (Exception) + { + // JSON parsing or property access failed — fall through to use original message + } + } + + var message = !string.IsNullOrEmpty(detailedMessage) ? detailedMessage : ex.Message; + var wrappedException = new AzPSCloudException(message, message, ex) + { + Request = ex.Request, + Response = ex.Response, + }; + + if (!string.IsNullOrEmpty(ex.Body?.Error?.Code)) + { + wrappedException.Data["CloudErrorCode"] = 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 6675e718f585..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; @@ -54,15 +55,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 +67,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(); } /// @@ -125,12 +112,13 @@ 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"))) + (!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 CreateExceptionWithDescriptiveErrorMessage(ex); + throw ErrorResponseExceptionHelper.CreateFrom(ex); } } @@ -141,19 +129,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 +145,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); } /// @@ -194,12 +168,12 @@ 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'."); } - throw CreateExceptionWithDescriptiveErrorMessage(ex); + throw ErrorResponseExceptionHelper.CreateFrom(ex); } } @@ -235,21 +209,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..e3ec63caedf4 100644 --- a/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs +++ b/src/Sql/Sql/Server/Services/AzureSqlDeletedServerAdapter.cs @@ -13,7 +13,9 @@ // ---------------------------------------------------------------------------------- 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; using Microsoft.Azure.Management.Sql.Models; using System; @@ -61,7 +63,16 @@ public DeletedServer GetDeletedServer(string location, string serverName, string } catch (ErrorResponseException ex) { - throw GetImprovedErrorResponseException(ex, location, serverName); + if (ex.Response?.StatusCode == System.Net.HttpStatusCode.NotFound) + { + var notFoundMessage = string.Format(Properties.Resources.DeletedServerNotFoundInLocation, serverName, location); + throw new AzPSResourceNotFoundCloudException( + notFoundMessage, + notFoundMessage, + ex); + } + + throw ErrorResponseExceptionHelper.CreateFrom(ex); } } @@ -110,31 +121,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