Make ProblemDetailsDefaults public

[Tornhoof]

Background and Motivation

Replaces https://github.com/dotnet/aspnetcore/issues/47822

Problem Details (RFC 7807) is a standardized way to communicate details for errors in HTTP Responses. In ASP.NET Core this is surfaced by the public type ProblemDetails and related types. The RFC states, that an URL in Type is the canonical way to identify the details, a list of default URLs for the common Status Codes is available in ProblemDetailsDefaults, this type also includes a method to apply defaults to ProblemDetails, but the type is internal. There is infrastructure available to write serialized ProblemDetails into responses, but it is designed around having access to HttpContext. If a developer wants to use the Problem Details without an active HttpRequest, he can't apply the nice defaults via the above mentioned type, instead need to maintain their own list and logic. The URLs currently point to the most recent HTTP rfc, this change was done for 8.0 via PR https://github.com/dotnet/aspnetcore/pull/43232

So I propose to make the type public.

Proposed API

// Microsoft.AspNetCore.Http.Extensions.dll (ProblemDetailsDefaults is currently in multiple assemblies, but they all depend on Http.Extensions)
namespace Microsoft.AspNetCore.Http;

- internal static class ProblemDetailsDefaults
+ public static class ProblemDetailsDefaults
{
-  public static readonly Dictionary<int, (string Type, string Title)> Defaults;
+  public static readonly IReadOnlyDictionary<int, (string Type, string Title)> Defaults;
   public static void Apply(ProblemDetails problemDetails, int? statusCode);
}

The type is changed to public, and the type of the static readonly field was changed to IReadOnlyDictionary, to prevent accidental changes.

Usage Examples

var pd = new ProblemDetails();
ProblemDetailDefaults.Apply(pd, 200);

Alternative Designs

@halter73 suggested in https://github.com/dotnet/aspnetcore/issues/47822 that the name Apply might not be descriptive enough and/or to add it to the ProblemDetails type directly named ApplyStatusCodeDefaults

namespace Microsoft.AspNetCore.Mvc;

public class ProblemDetails
{
+     public void ApplyStatusCodeDefaults(int? statusCode);
}

There is also the interface IProblemDetailsService where the same method could be added:

namespace Microsoft.AspNetCore.Mvc;

public interface IProblemDetailsService
{
+     public void ApplyStatusCodeDefaults(ProblemDetails problemDetails, int? statusCode);
}

Risks

• Making the static type public is the easiest, with few risks as the type is internal at the moment, but it has risks as the type is included in multiple assemblies.
• Adding it to the ProblemDetails as a member requires more thought, as there are already derived types of ProblemDetails and defaults might mean something else for them, the method might need to be virtual.
• The interface is just for completeness here, as this is a proper breaking change (without some default interface implementation workarounds), while I doubt that many devs supply their own implementation of that interface, I don't think it is worth it.

[campbellwray]

@Tornhoof, I was following your original issue, and was not aware of this:

There is infrastructure available to write serialized ProblemDetails into responses, but it is designed around having access to HttpContext.

Would you mind providing some more information, as I believe I have access to an HttpContext in my implementation.

Currently I am creating a ProblemDetails object and using (a public copy of) ProblemDetailsDefaults to add the Type and other details automatically, I was wondering if it might be possible to use an existing language feature to perform this task.

Thanks.

[Tornhoof]

Would you mind providing some more information, as I believe I have access to an HttpContext in my implementation.

There is the afore mentioned IProblemDetailsService which takes a ProblemDetailsContext, this one takes both the HttpContext and the ProblemDetails and uses IProblemDetailsWriter to serialize the details into JSON. See https://github.com/dotnet/aspnetcore/blob/main/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs for details.

[mitchdenny]

Somewhat related to this is an API review that we've just created which asks for the ability to make the ProblemDetails on ProblemDetailsContext settable:

https://github.com/dotnet/aspnetcore/issues/47633

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

Thanks for the API proposal!

I don't think we're going to expose the type and title as a ValueTuple, but we might expose it if we defined a proper type for it. We'd then need to figure out what to name it though. I'd be tempted to go with IReadOnlyDictionary<int, ProblemDetails> and leave most of the properties uninitialized, but we can't put mutable values in there.

Is exposing the IReadOnlyDictionary important? Or is a method populating an existing ProblemDetails sufficient?

[Tornhoof]

From my POV the method would be enough.

[halter73]

API Review Notes:

• We still don't want to expose the type and title as a ValueTuple.
• You could just copy the dictionary yourself into your code.
• Would anyone other than @Tornhoof and @campbellwray use this? It's not clear that they would. Might be nice if you were serializing to something other than the HttpResponse using the writer though.
• Would this lock us into supporting something we want to change in the future? We don't think so, but you never know.

API needs more interest to warrant the public API change. We were the closest to approving the instance method without the dictionary, but we decided against approving any new API at this time.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.