Skip to content

Commit

Permalink
[AOT] Fixing the problems with ProblemDetails (#45646)
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesNK committed Jan 3, 2023
1 parent 7b9537f commit 4e48ef7
Show file tree
Hide file tree
Showing 8 changed files with 215 additions and 73 deletions.
10 changes: 9 additions & 1 deletion src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetails.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Http;

Expand All @@ -12,6 +13,8 @@ namespace Microsoft.AspNetCore.Mvc;
[JsonConverter(typeof(ProblemDetailsJsonConverter))]
public class ProblemDetails
{
private readonly IDictionary<string, object?> _extensions = new Dictionary<string, object?>(StringComparer.Ordinal);

/// <summary>
/// A URI reference [RFC3986] that identifies the problem type. This specification encourages that, when
/// dereferenced, it provide human-readable documentation for the problem type
Expand Down Expand Up @@ -59,5 +62,10 @@ public class ProblemDetails
/// In particular, complex types or collection types may not round-trip to the original type when using the built-in JSON or XML formatters.
/// </remarks>
[JsonExtensionData]
public IDictionary<string, object?> Extensions { get; } = new Dictionary<string, object?>(StringComparer.Ordinal);
public IDictionary<string, object?> Extensions
{
[RequiresUnreferencedCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")]
[RequiresDynamicCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")]
get => _extensions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,40 @@ public class HttpValidationProblemDetailsJsonConverterTest
{
private static JsonSerializerOptions JsonSerializerOptions => new JsonOptions().SerializerOptions;

[Fact]
public void Write_Works()
{
var converter = new HttpValidationProblemDetailsJsonConverter();
var problemDetails = new HttpValidationProblemDetails();

problemDetails.Type = "https://tools.ietf.org/html/rfc9110#section-15.5.5";
problemDetails.Title = "Not found";
problemDetails.Status = 404;
problemDetails.Detail = "Product not found";
problemDetails.Instance = "http://example.com/products/14";
problemDetails.Extensions["traceId"] = "|37dd3dd5-4a9619f953c40a16.";
problemDetails.Errors.Add("key0", new[] { "error0" });
problemDetails.Errors.Add("key1", new[] { "error1", "error2" });

var ms = new MemoryStream();
var writer = new Utf8JsonWriter(ms);
converter.Write(writer, problemDetails, JsonSerializerOptions);
writer.Flush();

ms.Seek(0, SeekOrigin.Begin);
var document = JsonDocument.Parse(ms);
Assert.Equal(problemDetails.Type, document.RootElement.GetProperty("type").GetString());
Assert.Equal(problemDetails.Title, document.RootElement.GetProperty("title").GetString());
Assert.Equal(problemDetails.Status, document.RootElement.GetProperty("status").GetInt32());
Assert.Equal(problemDetails.Detail, document.RootElement.GetProperty("detail").GetString());
Assert.Equal(problemDetails.Instance, document.RootElement.GetProperty("instance").GetString());
Assert.Equal((string)problemDetails.Extensions["traceId"]!, document.RootElement.GetProperty("traceId").GetString());
var errorsElement = document.RootElement.GetProperty("errors");
Assert.Equal("error0", errorsElement.GetProperty("key0")[0].GetString());
Assert.Equal("error1", errorsElement.GetProperty("key1")[0].GetString());
Assert.Equal("error2", errorsElement.GetProperty("key1")[1].GetString());
}

[Fact]
public void Read_Works()
{
Expand Down
31 changes: 24 additions & 7 deletions src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Options;
Expand Down Expand Up @@ -48,20 +50,20 @@ public bool CanWrite(ProblemDetailsContext context)
}

[UnconditionalSuppressMessage("Trimming", "IL2026",
Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed and we need to fallback" +
"to reflection-based. The ProblemDetailsConverter is marked as RequiresUnreferencedCode already.")]
Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed. The property is annotated with a warning")]
[UnconditionalSuppressMessage("Trimming", "IL3050",
Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed and we need to fallback" +
"to reflection-based. The ProblemDetailsConverter is marked as RequiresDynamicCode already.")]
Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed. The property is annotated with a warning")]
public ValueTask WriteAsync(ProblemDetailsContext context)
{
var httpContext = context.HttpContext;
ProblemDetailsDefaults.Apply(context.ProblemDetails, httpContext.Response.StatusCode);
_options.CustomizeProblemDetails?.Invoke(context);

if (context.ProblemDetails.Extensions is { Count: 0 })
// Use source generation serialization in two scenarios:
// 1. There are no extensions. Source generation is faster and works well with trimming.
// 2. Native AOT. In this case only the data types specified on ProblemDetailsJsonContext will work.
if (context.ProblemDetails.Extensions is { Count: 0 } || !RuntimeFeature.IsDynamicCodeSupported)
{
// We can use the source generation in this case
return new ValueTask(httpContext.Response.WriteAsJsonAsync(
context.ProblemDetails,
ProblemDetailsJsonContext.Default.ProblemDetails,
Expand All @@ -74,7 +76,22 @@ public ValueTask WriteAsync(ProblemDetailsContext context)
contentType: "application/problem+json"));
}

// Additional values are specified on JsonSerializerContext to support some values for extensions.
// For example, the DeveloperExceptionMiddleware serializes its complex type to JsonElement, which problem details then needs to serialize.
[JsonSerializable(typeof(ProblemDetails))]
[JsonSerializable(typeof(JsonElement))]
[JsonSerializable(typeof(string))]
[JsonSerializable(typeof(decimal))]
[JsonSerializable(typeof(float))]
[JsonSerializable(typeof(double))]
[JsonSerializable(typeof(int))]
[JsonSerializable(typeof(long))]
[JsonSerializable(typeof(Guid))]
[JsonSerializable(typeof(Uri))]
[JsonSerializable(typeof(TimeSpan))]
[JsonSerializable(typeof(DateTime))]
[JsonSerializable(typeof(DateTimeOffset))]
internal sealed partial class ProblemDetailsJsonContext : JsonSerializerContext
{ }
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Diagnostics.RazorViews;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Json;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.FileProviders;
Expand All @@ -34,6 +37,7 @@ internal class DeveloperExceptionPageMiddlewareImpl
private readonly ExceptionDetailsProvider _exceptionDetailsProvider;
private readonly Func<ErrorContext, Task> _exceptionHandler;
private static readonly MediaTypeHeaderValue _textHtmlMediaType = new MediaTypeHeaderValue("text/html");
private readonly ExtensionsExceptionJsonContext _serializationContext;
private readonly IProblemDetailsService? _problemDetailsService;

/// <summary>
Expand All @@ -45,6 +49,7 @@ internal class DeveloperExceptionPageMiddlewareImpl
/// <param name="hostingEnvironment"></param>
/// <param name="diagnosticSource">The <see cref="DiagnosticSource"/> used for writing diagnostic messages.</param>
/// <param name="filters">The list of registered <see cref="IDeveloperPageExceptionFilter"/>.</param>
/// <param name="jsonOptions">The <see cref="JsonOptions"/> used for serialization.</param>
/// <param name="problemDetailsService">The <see cref="IProblemDetailsService"/> used for writing <see cref="ProblemDetails"/> messages.</param>
public DeveloperExceptionPageMiddlewareImpl(
RequestDelegate next,
Expand All @@ -53,6 +58,7 @@ public DeveloperExceptionPageMiddlewareImpl(
IWebHostEnvironment hostingEnvironment,
DiagnosticSource diagnosticSource,
IEnumerable<IDeveloperPageExceptionFilter> filters,
IOptions<JsonOptions>? jsonOptions = null,
IProblemDetailsService? problemDetailsService = null)
{
if (next == null)
Expand All @@ -77,15 +83,22 @@ public DeveloperExceptionPageMiddlewareImpl(
_diagnosticSource = diagnosticSource;
_exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _logger, _options.SourceCodeLineCount);
_exceptionHandler = DisplayException;
_serializationContext = CreateSerializationContext(jsonOptions?.Value);
_problemDetailsService = problemDetailsService;

foreach (var filter in filters.Reverse())
{
var nextFilter = _exceptionHandler;
_exceptionHandler = errorContext => filter.HandleExceptionAsync(errorContext, nextFilter);
}
}

private static ExtensionsExceptionJsonContext CreateSerializationContext(JsonOptions? jsonOptions)
{
// Create context from configured options to get settings such as PropertyNamePolicy and DictionaryKeyPolicy.
jsonOptions ??= new JsonOptions();
return new ExtensionsExceptionJsonContext(new JsonSerializerOptions(jsonOptions.SerializerOptions));
}

/// <summary>
/// Process an individual request.
/// </summary>
Expand Down Expand Up @@ -172,21 +185,7 @@ private async Task DisplayExceptionContent(ErrorContext errorContext)

if (_problemDetailsService != null)
{
var problemDetails = new ProblemDetails
{
Title = TypeNameHelper.GetTypeDisplayName(errorContext.Exception.GetType()),
Detail = errorContext.Exception.Message,
Status = httpContext.Response.StatusCode
};

problemDetails.Extensions["exception"] = new
{
Details = errorContext.Exception.ToString(),
Headers = httpContext.Request.Headers,
Path = httpContext.Request.Path.ToString(),
Endpoint = httpContext.GetEndpoint()?.ToString(),
RouteValues = httpContext.Features.Get<IRouteValuesFeature>()?.RouteValues,
};
var problemDetails = CreateProblemDetails(errorContext, httpContext);

await _problemDetailsService.WriteAsync(new()
{
Expand Down Expand Up @@ -214,6 +213,31 @@ await _problemDetailsService.WriteAsync(new()
}
}

[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Values set on ProblemDetails.Extensions are supported by the default writer.")]
[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "Values set on ProblemDetails.Extensions are supported by the default writer.")]
private ProblemDetails CreateProblemDetails(ErrorContext errorContext, HttpContext httpContext)
{
var problemDetails = new ProblemDetails
{
Title = TypeNameHelper.GetTypeDisplayName(errorContext.Exception.GetType()),
Detail = errorContext.Exception.Message,
Status = httpContext.Response.StatusCode
};

// Problem details source gen serialization doesn't know about IHeaderDictionary or RouteValueDictionary.
// Serialize payload to a JsonElement here. Problem details serialization can write JsonElement in extensions dictionary.
problemDetails.Extensions["exception"] = JsonSerializer.SerializeToElement(new ExceptionExtensionData
(
Details: errorContext.Exception.ToString(),
Headers: httpContext.Request.Headers,
Path: httpContext.Request.Path.ToString(),
Endpoint: httpContext.GetEndpoint()?.ToString(),
RouteValues: httpContext.Features.Get<IRouteValuesFeature>()?.RouteValues
), _serializationContext.ExceptionExtensionData);

return problemDetails;
}

private Task DisplayCompilationException(
HttpContext context,
ICompilationException compilationException)
Expand Down Expand Up @@ -328,3 +352,10 @@ private Task DisplayRuntimeException(HttpContext context, Exception ex)
return errorPage.ExecuteAsync(context);
}
}

internal record ExceptionExtensionData(string Details, IHeaderDictionary Headers, string Path, string? Endpoint, RouteValueDictionary? RouteValues);

[JsonSerializable(typeof(ExceptionExtensionData))]
internal sealed partial class ExtensionsExceptionJsonContext : JsonSerializerContext
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net.Http.Headers;
using Microsoft.AspNetCore.Mvc;
using System.Net.Http.Json;
using System.Text.Json;

namespace Microsoft.AspNetCore.Diagnostics.FunctionalTests;

Expand Down Expand Up @@ -49,5 +50,14 @@ public async Task DeveloperExceptionPage_ShowsProblemDetails_WhenHtmlNotAccepted
Assert.NotNull(body);
Assert.Equal(500, body.Status);
Assert.Contains("Demonstration exception", body.Detail);

var exceptionNode = (JsonElement)body.Extensions["exception"];
Assert.Contains("System.Exception: Demonstration exception.", exceptionNode.GetProperty("details").GetString());
Assert.Equal("application/json", exceptionNode.GetProperty("headers").GetProperty("Accept")[0].GetString());
Assert.Equal("localhost", exceptionNode.GetProperty("headers").GetProperty("Host")[0].GetString());
Assert.Equal("/", exceptionNode.GetProperty("path").GetString());
Assert.Equal("Endpoint display name", exceptionNode.GetProperty("endpoint").GetString());
Assert.Equal("Value1", exceptionNode.GetProperty("routeValues").GetProperty("routeValue1").GetString());
Assert.Equal("Value2", exceptionNode.GetProperty("routeValues").GetProperty("routeValue2").GetString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public void WriteWorks()
using Utf8JsonWriter writer = new(stream);

// Act
converter.Write(writer, problemDetails, null);
converter.Write(writer, problemDetails, new JsonSerializerOptions());

writer.Flush();
var json = Encoding.UTF8.GetString(stream.ToArray());
Expand Down
Loading

0 comments on commit 4e48ef7

Please sign in to comment.