From 7eec5ca7c560bbf3592adac5b686609cbb0aa945 Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Mon, 24 Feb 2025 00:03:55 +0100 Subject: [PATCH] Marshall JsonApiException thrown from JsonConverter such that the source pointer can be reconstructed (#1690) --- .../JsonConverters/ResourceObjectConverter.cs | 21 +++ .../Serialization/Request/JsonApiReader.cs | 4 + .../NotSupportedExceptionExtensions.cs | 91 +++++++++++ .../ResourceObjectConverterTests.cs | 47 +++++- .../SourcePointerInExceptionTests.cs | 143 ++++++++++++++++++ 5 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 src/JsonApiDotNetCore/Serialization/Request/NotSupportedExceptionExtensions.cs create mode 100644 test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/SourcePointerInExceptionTests.cs diff --git a/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs b/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs index 2f820175d2..8bc67c0196 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs @@ -1,7 +1,10 @@ +using System.Diagnostics.CodeAnalysis; using System.Reflection; +using System.Runtime.ExceptionServices; using System.Text.Json; using JetBrains.Annotations; using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; using JsonApiDotNetCore.Serialization.Objects; @@ -372,4 +375,22 @@ private protected virtual void WriteExtensionInAttributes(Utf8JsonWriter writer, private protected virtual void WriteExtensionInRelationships(Utf8JsonWriter writer, ResourceObject value) { } + + /// + /// Throws a in such a way that can reconstruct the source pointer. + /// + /// + /// The to throw, which may contain a relative source pointer. + /// + [DoesNotReturn] + [ContractAnnotation("=> halt")] + private protected static void CapturedThrow(JsonApiException exception) + { + ExceptionDispatchInfo.SetCurrentStackTrace(exception); + + throw new NotSupportedException(null, exception) + { + Source = "System.Text.Json.Rethrowable" + }; + } } diff --git a/src/JsonApiDotNetCore/Serialization/Request/JsonApiReader.cs b/src/JsonApiDotNetCore/Serialization/Request/JsonApiReader.cs index f565395969..9c0139d949 100644 --- a/src/JsonApiDotNetCore/Serialization/Request/JsonApiReader.cs +++ b/src/JsonApiDotNetCore/Serialization/Request/JsonApiReader.cs @@ -97,6 +97,10 @@ private Document DeserializeDocument(string requestBody) // https://github.com/dotnet/runtime/issues/50205#issuecomment-808401245 throw new InvalidRequestBodyException(_options.IncludeRequestBodyInErrors ? requestBody : null, null, exception.Message, null, null, exception); } + catch (NotSupportedException exception) when (exception.HasJsonApiException()) + { + throw exception.EnrichSourcePointer(); + } } private void AssertHasDocument([SysNotNull] Document? document, string requestBody) diff --git a/src/JsonApiDotNetCore/Serialization/Request/NotSupportedExceptionExtensions.cs b/src/JsonApiDotNetCore/Serialization/Request/NotSupportedExceptionExtensions.cs new file mode 100644 index 0000000000..70e44f5730 --- /dev/null +++ b/src/JsonApiDotNetCore/Serialization/Request/NotSupportedExceptionExtensions.cs @@ -0,0 +1,91 @@ +using System.Text.Json.Serialization; +using JsonApiDotNetCore.Errors; +using JsonApiDotNetCore.Serialization.Objects; + +namespace JsonApiDotNetCore.Serialization.Request; + +/// +/// A hacky approach to obtain the proper JSON:API source pointer from an exception thrown in a . +/// +/// +/// +/// This method relies on the behavior at +/// https://github.com/dotnet/runtime/blob/release/8.0/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.ReadCore.cs#L100, +/// which wraps a thrown and adds the JSON path to the outer exception message, based on internal reader state. +/// +/// +/// To take advantage of this, we expect a custom converter to throw a with a specially-crafted +/// and a nested containing a relative source pointer and a captured stack trace. Once +/// all of that happens, this class extracts the added JSON path from the outer exception message and converts it to a JSON:API pointer to enrich the +/// nested with. +/// +/// +internal static class NotSupportedExceptionExtensions +{ + private const string LeadingText = " Path: "; + private const string TrailingText = " | LineNumber: "; + + public static bool HasJsonApiException(this NotSupportedException exception) + { + return exception.InnerException is NotSupportedException { InnerException: JsonApiException }; + } + + public static JsonApiException EnrichSourcePointer(this NotSupportedException exception) + { + var jsonApiException = (JsonApiException)exception.InnerException!.InnerException!; + string? sourcePointer = GetSourcePointerFromMessage(exception.Message); + + if (sourcePointer != null) + { + foreach (ErrorObject error in jsonApiException.Errors) + { + if (error.Source == null) + { + error.Source = new ErrorSource + { + Pointer = sourcePointer + }; + } + else + { + error.Source.Pointer = sourcePointer + '/' + error.Source.Pointer; + } + } + } + + return jsonApiException; + } + + private static string? GetSourcePointerFromMessage(string message) + { + string? jsonPath = ExtractJsonPathFromMessage(message); + return JsonPathToSourcePointer(jsonPath); + } + + private static string? ExtractJsonPathFromMessage(string message) + { + int startIndex = message.IndexOf(LeadingText, StringComparison.Ordinal); + + if (startIndex != -1) + { + int stopIndex = message.IndexOf(TrailingText, startIndex, StringComparison.Ordinal); + + if (stopIndex != -1) + { + return message.Substring(startIndex + LeadingText.Length, stopIndex - startIndex - LeadingText.Length); + } + } + + return null; + } + + private static string? JsonPathToSourcePointer(string? jsonPath) + { + if (jsonPath != null && jsonPath.StartsWith('$')) + { + return jsonPath[1..].Replace('.', '/'); + } + + return null; + } +} diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/ResourceObjectConverterTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/ResourceObjectConverterTests.cs index 9dbdb5d68d..d5a63c0447 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/ResourceObjectConverterTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/ResourceObjectConverterTests.cs @@ -1,8 +1,10 @@ +using System.Net; using System.Text; using System.Text.Json; using FluentAssertions; using JetBrains.Annotations; using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.Middleware; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; @@ -186,7 +188,16 @@ public void Throws_for_request_body_with_extension_in_attributes_when_extension_ }; // Assert - action.Should().ThrowExactly().WithMessage("Failure requested from attributes."); + JsonApiException? exception = action.Should().ThrowExactly().WithInnerExceptionExactly().Which; + + exception.StackTrace.Should().Contain(nameof(ExtensionAwareResourceObjectConverter)); + exception.Errors.ShouldHaveCount(1); + + ErrorObject error = exception.Errors[0]; + error.StatusCode.Should().Be(HttpStatusCode.BadRequest); + error.Title.Should().Be("Failure requested from attributes."); + error.Source.ShouldNotBeNull(); + error.Source.Pointer.Should().Be("attributes/type-info:fail"); } [Fact] @@ -218,7 +229,16 @@ public void Throws_for_request_body_with_extension_in_relationships_when_extensi }; // Assert - action.Should().ThrowExactly().WithMessage("Failure requested from relationships."); + JsonApiException? exception = action.Should().ThrowExactly().WithInnerExceptionExactly().Which; + + exception.StackTrace.Should().Contain(nameof(ExtensionAwareResourceObjectConverter)); + exception.Errors.ShouldHaveCount(1); + + ErrorObject error = exception.Errors[0]; + error.StatusCode.Should().Be(HttpStatusCode.BadRequest); + error.Title.Should().Be("Failure requested from relationships."); + error.Source.ShouldNotBeNull(); + error.Source.Pointer.Should().Be("relationships/type-info:fail"); } [Fact] @@ -401,6 +421,7 @@ public void Writes_extension_in_response_body_when_extension_enabled_with_derive private sealed class ExtensionAwareResourceObjectConverter : ResourceObjectConverter { private const string ExtensionNamespace = "type-info"; + private const string ExtensionName = "fail"; private readonly IResourceGraph _resourceGraph; private readonly JsonApiRequestAccessor _requestAccessor; @@ -420,11 +441,18 @@ public ExtensionAwareResourceObjectConverter(IResourceGraph resourceGraph, JsonA private protected override void ValidateExtensionInAttributes(string extensionNamespace, string extensionName, ResourceType resourceType, Utf8JsonReader reader) { - if (extensionNamespace == ExtensionNamespace && IsTypeInfoExtensionEnabled && extensionName == "fail") + if (extensionNamespace == ExtensionNamespace && IsTypeInfoExtensionEnabled && extensionName == ExtensionName) { if (reader.GetBoolean()) { - throw new JsonException("Failure requested from attributes."); + CapturedThrow(new JsonApiException(new ErrorObject(HttpStatusCode.BadRequest) + { + Title = "Failure requested from attributes.", + Source = new ErrorSource + { + Pointer = $"attributes/{ExtensionNamespace}:{ExtensionName}" + } + })); } return; @@ -436,11 +464,18 @@ private protected override void ValidateExtensionInAttributes(string extensionNa private protected override void ValidateExtensionInRelationships(string extensionNamespace, string extensionName, ResourceType resourceType, Utf8JsonReader reader) { - if (extensionNamespace == ExtensionNamespace && IsTypeInfoExtensionEnabled && extensionName == "fail") + if (extensionNamespace == ExtensionNamespace && IsTypeInfoExtensionEnabled && extensionName == ExtensionName) { if (reader.GetBoolean()) { - throw new JsonException("Failure requested from relationships."); + CapturedThrow(new JsonApiException(new ErrorObject(HttpStatusCode.BadRequest) + { + Title = "Failure requested from relationships.", + Source = new ErrorSource + { + Pointer = $"relationships/{ExtensionNamespace}:{ExtensionName}" + } + })); } return; diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/SourcePointerInExceptionTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/SourcePointerInExceptionTests.cs new file mode 100644 index 0000000000..9216b4b896 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/UnitTests/Serialization/Extensions/SourcePointerInExceptionTests.cs @@ -0,0 +1,143 @@ +using System.Net; +using System.Text.Json; +using FluentAssertions; +using JetBrains.Annotations; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Errors; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Serialization.JsonConverters; +using JsonApiDotNetCore.Serialization.Objects; +using JsonApiDotNetCore.Serialization.Request; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging.Abstractions; +using TestBuildingBlocks; +using Xunit; + +namespace JsonApiDotNetCoreTests.UnitTests.Serialization.Extensions; + +public sealed class SourcePointerInExceptionTests +{ + private const string RequestBody = """ + { + "data": { + "type": "testResources", + "attributes": { + "ext-namespace:ext-name": "ignored" + } + } + } + """; + + [Fact] + public async Task Adds_source_pointer_to_JsonApiException_thrown_from_JsonConverter() + { + // Arrange + const string? relativeSourcePointer = null; + + var options = new JsonApiOptions(); + IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Build(); + var converter = new ThrowingResourceObjectConverter(resourceGraph, relativeSourcePointer); + var reader = new FakeJsonApiReader(RequestBody, options, converter); + var httpContext = new DefaultHttpContext(); + + // Act + Func action = async () => await reader.ReadAsync(httpContext.Request); + + // Assert + JsonApiException? exception = (await action.Should().ThrowExactlyAsync()).Which; + + exception.StackTrace.Should().Contain(nameof(ThrowingResourceObjectConverter)); + exception.Errors.ShouldHaveCount(1); + + ErrorObject error = exception.Errors[0]; + error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + error.Title.Should().Be("Extension error"); + error.Source.ShouldNotBeNull(); + error.Source.Pointer.Should().Be("/data"); + } + + [Fact] + public async Task Makes_source_pointer_absolute_in_JsonApiException_thrown_from_JsonConverter() + { + // Arrange + const string relativeSourcePointer = "relative/path"; + + var options = new JsonApiOptions(); + IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Build(); + var converter = new ThrowingResourceObjectConverter(resourceGraph, relativeSourcePointer); + var reader = new FakeJsonApiReader(RequestBody, options, converter); + var httpContext = new DefaultHttpContext(); + + // Act + Func action = async () => await reader.ReadAsync(httpContext.Request); + + // Assert + JsonApiException? exception = (await action.Should().ThrowExactlyAsync()).Which; + + exception.StackTrace.Should().Contain(nameof(ThrowingResourceObjectConverter)); + exception.Errors.ShouldHaveCount(1); + + ErrorObject error = exception.Errors[0]; + error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + error.Title.Should().Be("Extension error"); + error.Source.ShouldNotBeNull(); + error.Source.Pointer.Should().Be("/data/relative/path"); + } + + [UsedImplicitly(ImplicitUseTargetFlags.Members)] + private sealed class TestResource : Identifiable; + + private sealed class ThrowingResourceObjectConverter(IResourceGraph resourceGraph, string? relativeSourcePointer) + : ResourceObjectConverter(resourceGraph) + { + private readonly string? _relativeSourcePointer = relativeSourcePointer; + + private protected override void ValidateExtensionInAttributes(string extensionNamespace, string extensionName, ResourceType resourceType, + Utf8JsonReader reader) + { + var exception = new JsonApiException(new ErrorObject(HttpStatusCode.UnprocessableEntity) + { + Title = "Extension error" + }); + + if (_relativeSourcePointer != null) + { + exception.Errors[0].Source = new ErrorSource + { + Pointer = _relativeSourcePointer + }; + } + + CapturedThrow(exception); + } + } + + private sealed class FakeJsonApiReader : IJsonApiReader + { + private readonly string _requestBody; + + private readonly JsonSerializerOptions _serializerOptions; + + public FakeJsonApiReader(string requestBody, JsonApiOptions options, ResourceObjectConverter converter) + { + _requestBody = requestBody; + + _serializerOptions = new JsonSerializerOptions(options.SerializerOptions); + _serializerOptions.Converters.Add(converter); + } + + public Task ReadAsync(HttpRequest httpRequest) + { + try + { + JsonSerializer.Deserialize(_requestBody, _serializerOptions); + } + catch (NotSupportedException exception) when (exception.HasJsonApiException()) + { + throw exception.EnrichSourcePointer(); + } + + return Task.FromResult(null); + } + } +}