diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ProblemDetailsClientErrorFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ProblemDetailsClientErrorFactory.cs index 1bf1e3ac43..ff47a18fa6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ProblemDetailsClientErrorFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ProblemDetailsClientErrorFactory.cs @@ -2,12 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Infrastructure { internal class ProblemDetailsClientErrorFactory : IClientErrorFactory { + private static readonly string TraceIdentifierKey = "traceId"; private readonly ApiBehaviorOptions _options; public ProblemDetailsClientErrorFactory(IOptions options) @@ -28,6 +30,8 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { problemDetails.Title = errorData.Title; problemDetails.Type = errorData.Link; + + SetTraceId(actionContext, problemDetails); } return new ObjectResult(problemDetails) @@ -40,5 +44,11 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure }, }; } + + internal static void SetTraceId(ActionContext actionContext, ProblemDetails problemDetails) + { + var traceId = Activity.Current?.Id ?? actionContext.HttpContext.TraceIdentifier; + problemDetails.Extensions[TraceIdentifierKey] = traceId; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs index 4d64d9bb84..bddaaa2478 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.Logging; @@ -128,9 +129,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal return result; } - private static IActionResult ProblemDetailsInvalidModelStateResponse(ActionContext context) + internal static IActionResult ProblemDetailsInvalidModelStateResponse(ActionContext context) { - var result = new BadRequestObjectResult(new ValidationProblemDetails(context.ModelState)); + var problemDetails = new ValidationProblemDetails(context.ModelState) + { + Status = StatusCodes.Status400BadRequest, + }; + + ProblemDetailsClientErrorFactory.SetTraceId(context, problemDetails); + + var result = new BadRequestObjectResult(problemDetails); result.ContentTypes.Add("application/problem+json"); result.ContentTypes.Add("application/problem+xml"); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ProblemDetalsClientErrorFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ProblemDetalsClientErrorFactoryTest.cs index 65a53a8158..603b60e381 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ProblemDetalsClientErrorFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ProblemDetalsClientErrorFactoryTest.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Diagnostics; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Options; using Xunit; @@ -22,7 +24,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure })); // Act - var result = factory.GetClientError(new ActionContext(), clientError); + var result = factory.GetClientError(GetActionContext(), clientError); // Assert var objectResult = Assert.IsType(result); @@ -49,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure })); // Act - var result = factory.GetClientError(new ActionContext(), clientError); + var result = factory.GetClientError(GetActionContext(), clientError); // Assert var objectResult = Assert.IsType(result); @@ -61,5 +63,67 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure Assert.Null(problemDetails.Detail); Assert.Null(problemDetails.Instance); } + + [Fact] + public void GetClientError_UsesActivityId_ToSetTraceId() + { + // Arrange + using (new ActivityReplacer()) + { + var clientError = new UnsupportedMediaTypeResult(); + var factory = new ProblemDetailsClientErrorFactory(Options.Create(new ApiBehaviorOptions + { + ClientErrorMapping = + { + [415] = new ClientErrorData { Link = "Some link", Title = "Summary" }, + }, + })); + + // Act + var result = factory.GetClientError(GetActionContext(), clientError); + + // Assert + var objectResult = Assert.IsType(result); + Assert.Equal(new[] { "application/problem+json", "application/problem+xml" }, objectResult.ContentTypes); + var problemDetails = Assert.IsType(objectResult.Value); + + Assert.Equal(Activity.Current.Id, problemDetails.Extensions["traceId"]); + } + } + + [Fact] + public void GetClientError_UsesHttpContext_ToSetTraceIdIfActivityIdIsNotSet() + { + // Arrange + var clientError = new UnsupportedMediaTypeResult(); + var factory = new ProblemDetailsClientErrorFactory(Options.Create(new ApiBehaviorOptions + { + ClientErrorMapping = + { + [415] = new ClientErrorData { Link = "Some link", Title = "Summary" }, + }, + })); + + // Act + var result = factory.GetClientError(GetActionContext(), clientError); + + // Assert + var objectResult = Assert.IsType(result); + Assert.Equal(new[] { "application/problem+json", "application/problem+xml" }, objectResult.ContentTypes); + var problemDetails = Assert.IsType(objectResult.Value); + + Assert.Equal("42", problemDetails.Extensions["traceId"]); + } + + private static ActionContext GetActionContext() + { + return new ActionContext + { + HttpContext = new DefaultHttpContext + { + TraceIdentifier = "42", + } + }; + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs index 622a36e5ff..4ae53cd4d7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs @@ -2,6 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; +using System.Linq; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; @@ -97,5 +100,64 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert Assert.Same(expected, options.InvalidModelStateResponseFactory); } + + [Fact] + public void ProblemDetailsInvalidModelStateResponse_ReturnsBadRequestWithProblemDetails() + { + // Arrange + var actionContext = new ActionContext + { + HttpContext = new DefaultHttpContext { TraceIdentifier = "42" }, + }; + + // Act + var result = ApiBehaviorOptionsSetup.ProblemDetailsInvalidModelStateResponse(actionContext); + + // Assert + var badRequest = Assert.IsType(result); + Assert.Equal(new[] { "application/problem+json", "application/problem+xml" }, badRequest.ContentTypes.OrderBy(c => c)); + + var problemDetails = Assert.IsType(badRequest.Value); + Assert.Equal(400, problemDetails.Status); + } + + [Fact] + public void ProblemDetailsInvalidModelStateResponse_SetsTraceId() + { + // Arrange + using (new ActivityReplacer()) + { + var actionContext = new ActionContext + { + HttpContext = new DefaultHttpContext { TraceIdentifier = "42" }, + }; + + // Act + var result = ApiBehaviorOptionsSetup.ProblemDetailsInvalidModelStateResponse(actionContext); + + // Assert + var badRequest = Assert.IsType(result); + var problemDetails = Assert.IsType(badRequest.Value); + Assert.Equal(Activity.Current.Id, problemDetails.Extensions["traceId"]); + } + } + + [Fact] + public void ProblemDetailsInvalidModelStateResponse_SetsTraceIdFromRequest_IfActivityIsNull() + { + // Arrange + var actionContext = new ActionContext + { + HttpContext = new DefaultHttpContext { TraceIdentifier = "42" }, + }; + + // Act + var result = ApiBehaviorOptionsSetup.ProblemDetailsInvalidModelStateResponse(actionContext); + + // Assert + var badRequest = Assert.IsType(result); + var problemDetails = Assert.IsType(badRequest.Value); + Assert.Equal("42", problemDetails.Extensions["traceId"]); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/ActivityReplacer.cs b/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/ActivityReplacer.cs new file mode 100644 index 0000000000..f7bc9d8193 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/ActivityReplacer.cs @@ -0,0 +1,25 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc +{ + public class ActivityReplacer : IDisposable + { + private readonly Activity _activity; + + public ActivityReplacer() + { + _activity = new Activity("Test"); + _activity.Start(); + } + + public void Dispose() + { + Debug.Assert(Activity.Current == _activity); + _activity.Stop(); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs index 127d612fdd..c30dd30750 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Net; using System.Net.Http; @@ -35,37 +36,48 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ActionsReturnBadRequest_WhenModelStateIsInvalid() { // Arrange - var contactModel = new Contact + using (new ActivityReplacer()) { - Name = "Abc", - City = "Redmond", - State = "WA", - Zip = "Invalid", - }; - var contactString = JsonConvert.SerializeObject(contactModel); - - // Act - var response = await Client.PostAsJsonAsync("/contact", contactModel); - - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); - Assert.Equal("application/problem+json", response.Content.Headers.ContentType.MediaType); - var problemDetails = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); - Assert.Collection( - problemDetails.Errors.OrderBy(kvp => kvp.Key), - kvp => + var contactModel = new Contact { - Assert.Equal("Name", kvp.Key); - var error = Assert.Single(kvp.Value); - Assert.Equal("The field Name must be a string with a minimum length of 5 and a maximum length of 30.", error); - }, - kvp => - { - Assert.Equal("Zip", kvp.Key); - var error = Assert.Single(kvp.Value); - Assert.Equal("The field Zip must match the regular expression '\\d{5}'.", error); - } - ); + Name = "Abc", + City = "Redmond", + State = "WA", + Zip = "Invalid", + }; + var contactString = JsonConvert.SerializeObject(contactModel); + + // Act + var response = await Client.PostAsJsonAsync("/contact", contactModel); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + Assert.Equal("application/problem+json", response.Content.Headers.ContentType.MediaType); + var problemDetails = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.Collection( + problemDetails.Errors.OrderBy(kvp => kvp.Key), + kvp => + { + Assert.Equal("Name", kvp.Key); + var error = Assert.Single(kvp.Value); + Assert.Equal("The field Name must be a string with a minimum length of 5 and a maximum length of 30.", error); + }, + kvp => + { + Assert.Equal("Zip", kvp.Key); + var error = Assert.Single(kvp.Value); + Assert.Equal("The field Zip must match the regular expression '\\d{5}'.", error); + } + ); + + Assert.Collection( + problemDetails.Extensions, + kvp => + { + Assert.Equal("traceId", kvp.Key); + Assert.Equal(Activity.Current.Id, kvp.Value); + }); + } } [Fact] @@ -253,21 +265,31 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests [Fact] public async Task ClientErrorResultFilterExecutesForStatusCodeResults() { - // Act - var response = await Client.GetAsync("/contact/ActionReturningStatusCodeResult"); + using (new ActivityReplacer()) + { + // Act + var response = await Client.GetAsync("/contact/ActionReturningStatusCodeResult"); - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); - var content = await response.Content.ReadAsStringAsync(); - var problemDetails = JsonConvert.DeserializeObject(content); - Assert.Equal(404, problemDetails.Status); + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); + var content = await response.Content.ReadAsStringAsync(); + var problemDetails = JsonConvert.DeserializeObject(content); + Assert.Equal(404, problemDetails.Status); + Assert.Collection( + problemDetails.Extensions, + kvp => + { + Assert.Equal("traceId", kvp.Key); + Assert.Equal(Activity.Current.Id, kvp.Value); + }); + } } [Fact] public async Task SerializingProblemDetails_IgnoresNullValuedProperties() { // Arrange - var expected = new[] { "status", "title", "type" }; + var expected = new[] { "status", "title", "traceId", "type" }; // Act var response = await Client.GetAsync("/contact/ActionReturningStatusCodeResult"); @@ -310,7 +332,14 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("Error", validationProblemDetails.Title); Assert.Equal(400, validationProblemDetails.Status); - Assert.Equal("27", validationProblemDetails.Extensions["tracking-id"]); + Assert.Collection( + validationProblemDetails.Extensions, + kvp => + { + Assert.Equal("tracking-id", kvp.Key); + Assert.Equal("27", kvp.Value); + }); + Assert.Collection( validationProblemDetails.Errors, kvp => diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj index 1e7cb2f215..bce36312a4 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj @@ -10,6 +10,7 @@ + diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs index 9ee16921a6..2e34fbbe27 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Diagnostics; using System.Net; using System.Net.Http; using System.Net.Http.Headers; @@ -213,15 +214,23 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ProblemDetails_IsSerialized() { // Arrange - var expected = @"404Not Foundhttps://tools.ietf.org/html/rfc7231#section-6.5.4"; + using (new ActivityReplacer()) + { + var expected = "" + + "404" + + "Not Found" + + "https://tools.ietf.org/html/rfc7231#section-6.5.4" + + $"{Activity.Current.Id}" + + ""; - // Act - var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningClientErrorStatusCodeResult"); + // Act + var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningClientErrorStatusCodeResult"); - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); - var content = await response.Content.ReadAsStringAsync(); - XmlAssert.Equal(expected, content); + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); + var content = await response.Content.ReadAsStringAsync(); + XmlAssert.Equal(expected, content); + } } [Fact] @@ -244,16 +253,25 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ValidationProblemDetails_IsSerialized() { // Arrange - var expected = @"400One or more validation errors occurred. -The State field is required."; + using (new ActivityReplacer()) + { + var expected = "" + + "400" + + "One or more validation errors occurred." + + $"{Activity.Current.Id}" + + "" + + "The State field is required." + + "" + + ""; - // Act - var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningValidationProblem"); + // Act + var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningValidationProblem"); - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); - var content = await response.Content.ReadAsStringAsync(); - XmlAssert.Equal(expected, content); + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + var content = await response.Content.ReadAsStringAsync(); + XmlAssert.Equal(expected, content); + } } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs index d54cd3880c..b8b41c5f0a 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Diagnostics; using System.Net; using System.Net.Http; using System.Net.Http.Headers; @@ -188,15 +189,23 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ProblemDetails_IsSerialized() { // Arrange - var expected = @"404Not Foundhttps://tools.ietf.org/html/rfc7231#section-6.5.4"; + using (new ActivityReplacer()) + { + var expected = "" + + "404" + + "Not Found" + + "https://tools.ietf.org/html/rfc7231#section-6.5.4" + + $"{Activity.Current.Id}" + + ""; - // Act - var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningClientErrorStatusCodeResult"); + // Act + var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningClientErrorStatusCodeResult"); - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); - var content = await response.Content.ReadAsStringAsync(); - XmlAssert.Equal(expected, content); + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); + var content = await response.Content.ReadAsStringAsync(); + XmlAssert.Equal(expected, content); + } } [Fact] @@ -219,16 +228,25 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ValidationProblemDetails_IsSerialized() { // Arrange - var expected = @"400One or more validation errors occurred. -The State field is required."; + using (new ActivityReplacer()) + { + var expected = "" + + "400" + + "One or more validation errors occurred." + + $"{Activity.Current.Id}" + + "" + + "The State field is required." + + "" + + ""; - // Act - var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningValidationProblem"); + // Act + var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningValidationProblem"); - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); - var content = await response.Content.ReadAsStringAsync(); - XmlAssert.Equal(expected, content); + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + var content = await response.Content.ReadAsStringAsync(); + XmlAssert.Equal(expected, content); + } } [Fact]