-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix New-AzRoleAssignment error message to include service error details #29466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
VeryEarly
merged 5 commits into
Azure:release-2026-05-05
from
MaddyMicrosoft:fix/issue-19605-roleassignment-error-message
Apr 29, 2026
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b53d750
Fix #19605: Surface service error details for role assignment operations
MaddyMicrosoft 5d43e1f
Fix #19605: Replace dead Hyak exception handler in role definition o…
MaddyMicrosoft aaac5de
Fixes as per PR comments
MaddyMicrosoft f54bc43
Test fix as per code changes
MaddyMicrosoft 1d1edfa
Changes as per comments
MaddyMicrosoft File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
235 changes: 235 additions & 0 deletions
235
src/Resources/Resources.Test/UnitTests/AuthorizationErrorResponseExceptionHelperTests.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,235 @@ | ||
| // ---------------------------------------------------------------------------------- | ||
| // | ||
| // 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 FluentAssertions; | ||
| using Microsoft.Azure.Commands.Common.Exceptions; | ||
| using Microsoft.Azure.Commands.Resources.Helper; | ||
| using Microsoft.Azure.Management.Authorization.Models; | ||
| using Microsoft.Rest; | ||
| using Microsoft.WindowsAzure.Commands.ScenarioTest; | ||
| using System.Net.Http; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Azure.Commands.Resources.Test.UnitTests | ||
| { | ||
| public class AuthorizationErrorResponseExceptionHelperTests | ||
| { | ||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithStructuredBody_IncludesCodeAndMessage() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'Conflict'") | ||
| { | ||
| Body = new ErrorResponse(new ErrorDetail( | ||
| code: "RoleAssignmentExists", | ||
| message: "The role assignment already exists.")) | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Should().BeOfType<AzPSCloudException>(); | ||
| result.Message.Should().Contain("Operation returned an invalid status code 'Conflict'"); | ||
| result.Message.Should().Contain("RoleAssignmentExists"); | ||
| result.Message.Should().Contain("The role assignment already exists."); | ||
| result.InnerException.Should().Be(ex); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithRawResponseContentOnly_IncludesResponseContent() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'NotFound'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.NotFound), | ||
| "{\"error\":{\"code\":\"SubscriptionNotFound\",\"message\":\"The subscription could not be found.\"}}") | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Should().BeOfType<AzPSCloudException>(); | ||
| result.Message.Should().Contain("Operation returned an invalid status code 'NotFound'"); | ||
| result.Message.Should().Contain("SubscriptionNotFound"); | ||
| result.InnerException.Should().Be(ex); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithNoBodyOrResponse_FallsBackToOriginalMessage() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'"); | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Should().BeOfType<AzPSCloudException>(); | ||
| result.Message.Should().Be("Operation returned an invalid status code 'BadRequest'"); | ||
| result.InnerException.Should().Be(ex); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_PreservesRequestAndResponse() | ||
| { | ||
| var request = new HttpRequestMessageWrapper( | ||
| new HttpRequestMessage(HttpMethod.Put, "https://management.azure.com/test"), | ||
| "{}"); | ||
| var response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.Conflict), | ||
| "{}"); | ||
| var ex = new ErrorResponseException("Conflict") | ||
| { | ||
| Request = request, | ||
| Response = response, | ||
| Body = new ErrorResponse(new ErrorDetail(code: "X", message: "Y")) | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Request.Should().Be(request); | ||
| result.Response.Should().Be(response); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithParseableResponseContent_FormatsCodeAndDetail() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'NotFound'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.NotFound), | ||
| "{\"error\":{\"code\":\"SubscriptionNotFound\",\"message\":\"The subscription could not be found.\"}}") | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| // Should produce the same "<orig>. code: detail" shape as the structured-body branch, | ||
| // not dump the raw JSON blob. | ||
| result.Message.Should().Contain("SubscriptionNotFound: The subscription could not be found."); | ||
| result.Message.Should().NotContain("Response: {"); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithMalformedResponseContent_FallsBackToRawContent() | ||
| { | ||
| const string malformed = "this is not json {"; | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest), | ||
| malformed) | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Message.Should().Contain("Operation returned an invalid status code 'BadRequest'"); | ||
| // Short content is embedded as-is (after whitespace collapsing) in the fallback. | ||
| result.Message.Should().Contain($"Response: {malformed}"); | ||
| // Full body remains accessible on the wrapped Response for debugging. | ||
| result.Response.Content.Should().Be(malformed); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithJsonMissingErrorObject_FallsBackToRawContent() | ||
| { | ||
| const string content = "{\"unrelated\":\"value\"}"; | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest), | ||
| content) | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Message.Should().Contain($"Response: {content}"); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithLargeUnparseableContent_TruncatesInMessageButPreservesFullBody() | ||
| { | ||
| // 1500 chars, well over the 500-char display limit, with embedded newlines. | ||
| var largeContent = new string('a', 700) + "\n\n" + new string('b', 800); | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest), | ||
| largeContent) | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Message.Should().Contain("... (truncated)"); | ||
| result.Message.Should().NotContain("\n\n"); // whitespace collapsed | ||
| result.Message.Length.Should().BeLessThan(largeContent.Length); | ||
| // Full body must remain available on the wrapped response for debugging. | ||
| result.Response.Content.Should().Be(largeContent); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithBothBodyAndResponseContent_PrefersStructuredBody() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'Conflict'") | ||
| { | ||
| Body = new ErrorResponse(new ErrorDetail(code: "BodyCode", message: "Body message.")), | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.Conflict), | ||
| "{\"error\":{\"code\":\"ResponseCode\",\"message\":\"Response message.\"}}") | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Message.Should().Contain("BodyCode"); | ||
| result.Message.Should().Contain("Body message."); | ||
| result.Message.Should().NotContain("ResponseCode"); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithCodeOnlyInResponseContent_OmitsTrailingColon() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest), | ||
| "{\"error\":{\"code\":\"OnlyCode\"}}") | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Message.Should().EndWith("OnlyCode"); | ||
| result.Message.Should().NotContain("OnlyCode:"); | ||
| } | ||
|
|
||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CreateDescriptiveException_WithMessageOnlyInResponseContent_OmitsLeadingColon() | ||
| { | ||
| var ex = new ErrorResponseException("Operation returned an invalid status code 'BadRequest'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper( | ||
| new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest), | ||
| "{\"error\":{\"message\":\"Only the message was provided.\"}}") | ||
| }; | ||
|
|
||
| var result = AuthorizationErrorResponseExceptionHelper.CreateDescriptiveException(ex); | ||
|
|
||
| result.Message.Should().EndWith("Only the message was provided."); | ||
| result.Message.Should().NotContain(": Only the message"); | ||
| } | ||
| } | ||
| } |
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
112 changes: 112 additions & 0 deletions
112
src/Resources/Resources/Helper/AuthorizationErrorResponseExceptionHelper.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| // ---------------------------------------------------------------------------------- | ||
| // | ||
| // 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.Authorization.Models; | ||
| using Newtonsoft.Json.Linq; | ||
| using System; | ||
| using System.Text.RegularExpressions; | ||
|
|
||
| namespace Microsoft.Azure.Commands.Resources.Helper | ||
| { | ||
| /// <summary> | ||
| /// Helper class to convert <see cref="ErrorResponseException"/> from the Authorization SDK | ||
| /// into <see cref="AzPSCloudException"/> with descriptive error messages. | ||
| /// The SDK-generated <see cref="ErrorResponseException"/> only contains the HTTP status code | ||
| /// in its message; the actual service error details live on <c>ex.Body.Error</c> or | ||
| /// <c>ex.Response.Content</c>. This helper surfaces those details to the user. | ||
| /// </summary> | ||
| internal static class AuthorizationErrorResponseExceptionHelper | ||
| { | ||
| // Telemetry-safe placeholder used when no service-supplied error code is available. | ||
| // Avoids any risk of leaking PII via ex.Message if the SDK template ever changes. | ||
| private const string UnknownErrorCode = "UnknownAuthorizationError"; | ||
|
|
||
| // Maximum number of characters of raw response content embedded in the user-facing | ||
| // exception message. The full body remains available on ex.Response.Content for debugging. | ||
| private const int MaxRawContentLength = 500; | ||
|
|
||
| /// <summary> | ||
| /// Creates an <see cref="AzPSCloudException"/> from an <see cref="ErrorResponseException"/>, | ||
| /// extracting the service error details from either the structured Body or the raw response content. | ||
| /// </summary> | ||
| /// <param name="ex">The original <see cref="ErrorResponseException"/>.</param> | ||
| /// <returns>An <see cref="AzPSCloudException"/> with the descriptive error message.</returns> | ||
| internal static AzPSCloudException CreateDescriptiveException(ErrorResponseException ex) | ||
| { | ||
| string message; | ||
| string desensitizedMessage; | ||
|
|
||
| if (ex.Body?.Error != null) | ||
| { | ||
| message = $"{ex.Message}. {ex.Body.Error.Code}: {ex.Body.Error.Message}"; | ||
| desensitizedMessage = ex.Body.Error.Code; | ||
| } | ||
| else if (!string.IsNullOrEmpty(ex.Response?.Content)) | ||
| { | ||
| (message, desensitizedMessage) = ParseErrorContent(ex.Message, ex.Response.Content); | ||
| } | ||
| else | ||
| { | ||
| message = ex.Message; | ||
| desensitizedMessage = UnknownErrorCode; | ||
| } | ||
|
|
||
| return new AzPSCloudException(message, desensitizedMessage, ex) | ||
| { | ||
| Request = ex.Request, | ||
| Response = ex.Response, | ||
| }; | ||
| } | ||
|
|
||
| // Builds the (user-facing message, telemetry-safe code) pair from a raw response body. | ||
| // On a successful parse of the standard Azure error JSON shape | ||
| // ({ "error": { "code": "...", "message": "..." } }) we surface code+message; | ||
| // otherwise we fall back to embedding the raw content with an UnknownErrorCode tag. | ||
| private static (string Message, string Desensitized) ParseErrorContent(string original, string content) | ||
| { | ||
| try | ||
| { | ||
| var error = JObject.Parse(content)["error"] as JObject; | ||
| var code = error?.Value<string>("code"); | ||
| var detail = error?.Value<string>("message"); | ||
|
|
||
| if (!string.IsNullOrEmpty(code) || !string.IsNullOrEmpty(detail)) | ||
| { | ||
| var suffix = !string.IsNullOrEmpty(code) && !string.IsNullOrEmpty(detail) | ||
| ? $"{code}: {detail}" | ||
| : (code ?? detail); | ||
| return ($"{original}. {suffix}", string.IsNullOrEmpty(code) ? UnknownErrorCode : code); | ||
| } | ||
| } | ||
| catch (Exception) | ||
| { | ||
| // Fall through to raw-content fallback. | ||
| } | ||
|
|
||
| return ($"{original}. Response: {TruncateForDisplay(content)}", UnknownErrorCode); | ||
| } | ||
|
|
||
| // Collapses runs of whitespace and truncates overly long bodies so a multi-line/large | ||
| // service response doesn't flood the console. The full body is still available via | ||
| // AzPSCloudException.Response.Content for debugging. | ||
| private static string TruncateForDisplay(string content) | ||
| { | ||
| var collapsed = Regex.Replace(content.Trim(), @"\s+", " "); | ||
| return collapsed.Length <= MaxRawContentLength | ||
| ? collapsed | ||
| : collapsed.Substring(0, MaxRawContentLength) + "... (truncated)"; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.