How to use legacy error format along side new ProblemDetails format depending upon API Version? #1072

mhsimkin · 2024-03-01T19:14:35Z

mhsimkin
Mar 1, 2024

Hi.

I am porting a legacy API from .NET 4.X to .NET 8. As part of this process, I am adding API versioning and updating to use the new standards (ProblemDetails, etc).

I need to maintain input and output format fidelity to the existing API. I have already determined how to make the original API the default, when a version is not specified. I have also determined that I can provide my own InvalidModelStateResponseFactory to handle custom format of the 400 response when the model fails validation.

However, when the new V2 API, I don't want to use the custom format for the V2 API calls. I would like to use the ProblemDetails response type.

What is the recommended approach to use switch between the two response formats at runtime depending upon what version of the API was called?

Do I just do something like this:

private IActionResult InvalidModelStateResponseFactory(ActionContext context)
{
    if (version == 1)
    {
       return LegacyModelErrorFormat(context);
    }

    var pdf = context.HttpContext.RequestServices.GetRequiredServices<ProblemDetailsFactory>();
    return pdf.CreateValidationProblemDetails(context.HttpContext, context.ModelState);
}

Thanks.

Answered by commonsensesoftware

Mar 4, 2024

Ah... I see the issue. This is because the JsonSerializerContext is defined by a source code generator. The Error Object types are protected to the writer itself, but are internal for the JsonSerializerContext. Once you inherit from it, they aren't visible in your derived implementation. The types themselves are structs so you can't inherit from them (which might have been a sleezy trick). The only thing you really need is the JsonSerializerContext, but since it's internal and the only thing that uses it is IConfigureOptions<T>, which is also internal, there's no way to access it without resorting to Reflection. 🤮

Arguably, this is a design flaw. It wasn't really clear to me how this migh…

View full answer

commonsensesoftware · 2024-03-01T19:45:51Z

commonsensesoftware
Mar 1, 2024
Maintainer

There's certainly some complexity if you're running both scenarios in the same application, but it's possible. I haven't done this myself, but I'm pretty sure what would work for you is something like this:

Extend `ErrorObjectWriter`

using Asp.Versioning;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Options;
using System.Text.Json;
using System.Text.Json.Serialization;

public sealed class ConditionalErrorObjectWriter : ErrorObjectWriter
{
    public override bool CanWrite(ProblemDetailsContext context)
    {
        if (base.CanWrite(context))
        {
            return context.HttpContext.GetRequestedApiVersion() == ApiVersion.Default; // == new ApiVersion(1.0)
        }

        return false;
    }

    [JsonSerializable( typeof( ErrorObject ) )]
    internal sealed partial class ConditionalErrorObjectJsonContext : JsonSerializerContext
    {
    }
}

Define the Setup Options

using Microsoft.AspNetCore.Http.Json;
using Microsoft.Extensions.Options;

internal sealed class ConditionalErrorObjectJsonOptionsSetup : IConfigureOptions<JsonOptions>
{
    public void Configure( JsonOptions options ) =>
        options.SerializerOptions.TypeInfoResolverChain.Insert(
            0,
            ConditionalErrorObjectWriter.ConditionalErrorObjectJsonContext.Default );
}

Register in DI

builder.Services.TryAddEnumerable(
    ServiceDescriptor.Singleton<IConfigureOptions<JsonOptions>, ConditionalErrorObjectJsonOptionsSetup>() );
builder.Services.AddControllers();
builder.Services.TryAddEnumerable( ServiceDescriptor.Singleton<IProblemDetailsWriter, ConditionalErrorObjectWriter>() );
builder.Services.AddProblemDetails();
builder.Services.ApiVersioning().AddMvc();

The IProblemDetailsService uses the Chain of Responsibility pattern and stops at the first matching IProblemDetailsWriter. I believe this will get you what you want. Your custom IProblemDetailsWriter will be evaluated first. If all other conditions are met and the request is for api-version=1.0, then the writer matches and the legacy Error Object is returned; otherwise, things fall through to the next writer (e.g. 2.0+).

0 replies

mhsimkin · 2024-03-01T20:23:37Z

mhsimkin
Mar 1, 2024
Author

@commonsensesoftware, thank you.

Not a lot of complexity, as the existing service only has one controller with one method, but two routes configured. Easy enough. I just know there are changes coming later this year where I will need to extended the API.

Since I currently porting this to .NET 8 from 4.7+ and migrating from RavenDB to Sqllite/SqlExpress and I have the time with no hard deadline, now is the time for me to get this code structured for the future changes. Including updating the code to use the new problem details format for model/request validation, exceptions.

A few questions:

I will need to implement the WriteAsync method on the ConditionalErrorObjectWriter to output my legacy format. Correct?
The ErrorObjectWriter class doesn't have a property called ConditionalErrorObjectJsonContext. Is this something I will need to implement.

Can you please let me know where I can read more about how ProblemDetails is implemented in .Net 8.

Thanks

-marc

1 reply

commonsensesoftware Mar 2, 2024
Maintainer

Oops! I goofed on the answer and I've updated it (slightly). I guess I didn't paste something.

You don't need to implement WriteAsync. I provided ErrorObjectWriter a while back for this interop scenario. It will do all of the work. The only thing you have to change is the extra condition in CanWrite a la override.
I goofed. All you need to do is include the partial class definition (see above) and the JSON source code generator will do the rest. This optimizes the serialization process. The JSON setup is how the infrastructure is configured so when it sees ErrorObject it knows how to serialize it.

If you really want to implement WriteAsync, you could. It's incredibly simple:

aspnet-api-versioning/src/AspNetCore/WebApi/src/Asp.Versioning.Http/ErrorObjectWriter.cs

Line 47 in ef0aa08

public virtual ValueTask WriteAsync( ProblemDetailsContext context )

The reason you need a ConditionalErrorObjectJsonContext is because I presumed that the only reason anyone would ever want to customize this behavior is to add (or perhaps remove) part of the serialization, which requires an alternate options. Conflicting sets of options would be problematic. You have a rare case where you have custom matching rules, but the same serialization. It's a small about of extra work, but not much.

aspnet-api-versioning/src/AspNetCore/WebApi/src/Asp.Versioning.Http/DependencyInjection/IServiceCollectionExtensions.cs

Line 182 in ef0aa08

private static void TryAddErrorObjectJsonOptions( IServiceCollection services )

mhsimkin · 2024-03-04T18:45:40Z

mhsimkin
Mar 4, 2024
Author

My apologues for the delayed response.

When I drop the above code into my project (just copy and paste, no changes) and compile, I'm getting the following errors:

1>  ...\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\ConditionalErrorObjectJsonContext.ErrorDetail.g.cs(20,135,20,146): error CS0053: Inconsistent accessibility: property type 'JsonTypeInfo<ErrorObjectWriter.ErrorDetail>' is less accessible than property 'ConditionalErrorObjectWriter.ConditionalErrorObjectJsonContext.ErrorDetail'
1>  ...\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\ConditionalErrorObjectJsonContext.ErrorObject.g.cs(20,135,20,146): error CS0053: Inconsistent accessibility: property type 'JsonTypeInfo<ErrorObjectWriter.ErrorObject>' is less accessible than property 'ConditionalErrorObjectWriter.ConditionalErrorObjectJsonContext.ErrorObject'
1>  ...\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\ConditionalErrorObjectJsonContext.InnerError.g.cs(20,134,20,144): error CS0053: Inconsistent accessibility: property type 'JsonTypeInfo<ErrorObjectWriter.InnerError>' is less accessible than property 'ConditionalErrorObjectWriter.ConditionalErrorObjectJsonContext.InnerError'
1>  ...\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\ConditionalErrorObjectJsonContext.NullableInnerError.g.cs(20,135,20,153): error CS0053: Inconsistent accessibility: property type 'JsonTypeInfo<ErrorObjectWriter.InnerError?>' is less accessible than property 'ConditionalErrorObjectWriter.ConditionalErrorObjectJsonContext.NullableInnerError'

I am going to assume that this has something to do with STJ converting the object serialization code at compile time and avoid the reflection based serialization. (As an aside, these errors are why I haven't turned this on for my own projects. Never seem to have figured out why there are errors being raised.).

Thanks

-marc

3 replies

commonsensesoftware Mar 4, 2024
Maintainer

Ah... I see the issue. This is because the JsonSerializerContext is defined by a source code generator. The Error Object types are protected to the writer itself, but are internal for the JsonSerializerContext. Once you inherit from it, they aren't visible in your derived implementation. The types themselves are structs so you can't inherit from them (which might have been a sleezy trick). The only thing you really need is the JsonSerializerContext, but since it's internal and the only thing that uses it is IConfigureOptions<T>, which is also internal, there's no way to access it without resorting to Reflection. 🤮

Arguably, this is a design flaw. It wasn't really clear to me how this might ever be extended. The correct, long-term library solution here is to simply provide a way to access or create the associated JsonSerializerContext; for example, some method or property on ErrorObjectWriter. That would certainly address your scenario. I'll queue that up for the next release. Unfortunately, that affects the public surface area and will require a minor version bump (but, that's not really a huge deal).

In the meantime, the best, most flexible option is to simply fork the entire implementation of ErrorObjectWriter verbatim. It's up to you if you want to use an alternate name (e.g. ConditionalErrorObjectWriter). Your implementation can be sealed and even internal if you like. No need for virtual members. The only difference in the fork will be the condition in CanWrite (as shown above in the original override). It's a bit unfortunate, but this approach seems to be the one with least amount of other weirdness. This type is already for backcompat. I don't expect this to ever change so you shouldn't have to worry about keeping your fork up-to-date. The only way I foresee that happening would be a bug in the implementation. Possible, but unlikely. Once there is a way to access ErrorObjectWriter.JsonSerializerContext in some future release, you can opt to replace the fork with the originally proposed subclass if you wish. The net result will be same though.

I don't have a better answer at the moment, but that should unblock you and get you back on your way.

Answer selected by mhsimkin

mhsimkin Mar 4, 2024
Author

Hi Chris (@commonsensesoftware).

Thank you.

If I understand everything. Since my ConditionalErrorObjectWriter is derived from ErrorObjectWriter, if my overridden CanWrite method returns false, than the next ErrorObjectWriter in the chain will be called. Now, if CanWrite returns true, the WriteAsync implementation of ConditionalErrorObjectWriter will be called, which is the base class implementation of WriteAsync. That method will still put out the RFC version of the problem details.

Below is a sanitized response from the current API. It always contains the same fields whether an error occurred or not. The input to the API call is a list of IDs. It is possible that one or more ids are invalid, but the rest are valid. As a result, the response will contain items inside the recommendations array and the invalid ids in the errors array.

{
    "algocd": "some string",
    "recommendations": []
    "errors": [],
    "isdefault": true,
    "isimported": false,
    "server": "ProductionServer",
    "setname": "application generated",
    "updateTimes": {}
}

I haven't figured out how this will be handled in the V2 API, however, it would probably be similar, but the format of the errors array would resemble a problem detail.

I'm beginning to think I might be better off just surpassing the new model invalid behavior by setting SuppressModelStateInvalidFilter = true and just using my own logic. I guess I need to finish the port and run my test suite. Then I can review the response being sent back, and decide how to handle exceptions versus bad input data.

If I decided that using a custom ErrorObjectWriter is the correct approach, I think I would be better off just implementing the IProblemsDetailsWriter.

Sorry for sending you down a rabbit hole,

Thank you for all the help.

commonsensesoftware Mar 4, 2024
Maintainer

It sounds like you have a mixture of valid and invalid results in a single response. In this case, neither Problem Details nor Error Object is probably the correct output. If you want to have an error look like Problem Details or an Error Object, there's nothing wrong with that.

A side note about Problem Details is that it differs from Error Object by more than just its JSON attributes and structure. Problem Details is always returned with the base media type application/problem so that a client knows what they are getting. It might otherwise be ambiguous. application/problem+json and application/problem+xml are the well-known suffix types, but Problem Details are not limited to those formats. The Error Object, on the other hand, is derived from the OData protocol (if you didn't know). It can also be represented as JSON or XML, but it does not have well-defined media type. A client has to assume that if they receive content in an unsuccessful response, that it should be an Error Object, but there is no guarantee of that.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

How to use legacy error format along side new ProblemDetails format depending upon API Version? #1072

Uh oh!

{{title}}

Uh oh!

Replies: 3 comments 4 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

How to use legacy error format along side new ProblemDetails format depending upon API Version? #1072

Uh oh!

mhsimkin Mar 1, 2024

Replies: 3 comments · 4 replies

Uh oh!

Uh oh!

commonsensesoftware Mar 1, 2024 Maintainer

Extend ErrorObjectWriter

Define the Setup Options

Register in DI

Uh oh!

mhsimkin Mar 1, 2024 Author

Uh oh!

commonsensesoftware Mar 2, 2024 Maintainer

Uh oh!

Uh oh!

mhsimkin Mar 4, 2024 Author

Uh oh!

commonsensesoftware Mar 4, 2024 Maintainer

Uh oh!

mhsimkin Mar 4, 2024 Author

Uh oh!

commonsensesoftware Mar 4, 2024 Maintainer

mhsimkin
Mar 1, 2024

Replies: 3 comments 4 replies

commonsensesoftware
Mar 1, 2024
Maintainer

Extend `ErrorObjectWriter`

mhsimkin
Mar 1, 2024
Author

commonsensesoftware Mar 2, 2024
Maintainer

mhsimkin
Mar 4, 2024
Author

commonsensesoftware Mar 4, 2024
Maintainer

mhsimkin Mar 4, 2024
Author

commonsensesoftware Mar 4, 2024
Maintainer