Allow using the DefaultProblemDetailsFactory without adding MVC Core

[Carl-Hugo]

Summary

I want to use the DefaultProblemDetailsFactory class without calling the AddMvcCore method.

Motivation and goals

I aim to leverage a ProblemDetailsFactory implementation (say the DefaultProblemDetailsFactory class) without adding MVC services to a Minimal API project.

More context: I'm updating older code to support .NET 7-8 that uses the ProblemDetailsFactory to generate ProblemDetails instances. The workaround was to call AddMvcCore then, which was kinda ok since there was only MVC before. However, now that we have Minimal APIs, adding MVC dependencies to a non-MVC project makes no sense.

Here are a few facts:

• The visibility modifier of the DefaultProblemDetailsFactory class is internal. See DefaultProblemDetailsFactory.cs for more info.
• The only way I found to add the DefaultProblemDetailsFactory to the container is by calling the AddMvcCore method. See MvcCoreServiceCollectionExtensions.cs for more info.
• In a non-MVC project, the AddMvcCore method adds a lot of dependencies for no reason.
• The newer AddProblemDetails method does not register any ProblemDetailsFactory with the container. See ProblemDetailsServiceCollectionExtensions.cs for more info.
• The IProblemDetailsService interface does not directly use the ProblemDetailsFactory, yet leveraging the factory to create an instance of the ProblemDetails class before setting the ProblemDetailsContext would allow consumers to reuse .NET building blocks, making the creation of the objects more "standard" and in line with other parts of .NET (a.k.a. reusing the registered ProblemDetailsFactory implementation). For example, adding the traceId and leveraging the ApiBehaviorOptions.ClientErrorMapping property to get/set some default values.
• I find it a little confusing that the ProblemDetailsFactory and ProblemDetails classes are part of MVC while their reach now goes beyond MVC.
  • The ProblemDetailsContext class references the ProblemDetails class.
  • The IProblemDetailsService interface uses the ProblemDetailsContext class.
  • Both ProblemDetailsContext and IProblemDetailsService are not part of MVC.

In scope

Allow registering the DefaultProblemDetailsFactory with the container without calling the AddMvcCore method.

• This could be as simple as changing the DefaultProblemDetailsFactory class visibility to public and letting the non-MVC consumers add the binding themselves.
• Another option would be to create a public extension method named AddDefaultProblemDetailsFactory that adds the binding and have AddMvcCore call this new method instead.
• Another option would be to add a TryAddSingleton<ProblemDetailsFactory, DefaultProblemDetailsFactory>() call in the AddProblemDetails method to register the DefaultProblemDetailsFactory on top of the IProblemDetailsService.

Or any other ways you think would be best. I am ok with creating a PR if there is some interest.

Out of scope

• Everything else.

Risks / unknowns

I don't see that many risks, if any.

[captainsafia]

@Carl-Hugo Thanks for filing this issue!

The newer ProblemDetailsService used by minimal APIs provides a slightly different experience for modifying the generated problem details. For example, if you'd like to modify the problem details that are produced globally by your application, you'd do something like this:

        builder.Services.AddProblemDetails(options =>
            options.CustomizeProblemDetails = ctx =>
                    ctx.ProblemDetails.Extensions.Add("nodeId", Environment.MachineName));

Based on some of what I'm seeing about how your interacting with the problem details type here this might be the best option?

[ghost]

Hi @Carl-Hugo. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[Carl-Hugo]

Hi @captainsafia, thanks for the suggestion, which sounds great to globally extend what .NET handles automatically or for a user of my library to extend it. However, in my case, I want to align with the default experience, hence tapping into the existing pieces, including the new ProblemDetailsService, but I must also customize the ProblemDetails object per exception type (all of this starts from a middleware registered in the alternate exception handling pipeline). For example, if the code works with a ProductNotFoundException that has a ProductId property, I want to add that value to the Extensions property of the ProblemDetails object. I must have a ProblemDetails instance to achieve that because of how the ProblemDetailsService class works.

The "issue" with the ProblemDetailsService is that its WriteAsync method takes a ProblemDetailsContext object which forces me to create a ProblemDetails instance, so I'm back to square one and still need to create the ProblemDetails object manually or hopefully use the ProblemDetailsFactory instead, tapping into the framework's logic, including the configuration you are talking about, the ProblemDetailsDefaults values and the configurable ClientErrorMapping from ApiBehaviorOptions.

An example of the convenience of using the factory is that the DefaultProblemDetailsFactory class adds the traceId field (the key format does not consider the serializer configuration, BTW). I could copy-paste the code, yet to create an experience as aligned with the native features as possible it would be better to directly tap into the .NET pieces, like the DefaultProblemDetailsFactory class (or anything else that allows doing the same thing). Why do I want this? If .NET improves things, I want the library to enable and leverage that seamlessly. Moreover, this makes my code as faithful to the .NET behavior as it can be, leading to a similar (see even the same) experience between what .NET handles and what my middleware handles. Not doing this may result in two different experiences at some point, which I'd like to avoid. The goal of the library is to centralize and uniformize exception handling. One could customize the behaviors, but by default, I want to mimic the .NET ones.

The ProblemDetails space is a bit confusing tbh; you have duplicated the functionalities, yet not completely, etc. Is the .NET team considering consolidating those two "branches" that solve the same problem into a single unified system?

Please let me know if you want more info.

[jeremy-allocate]

I would also like to see this situation improve. We use a global exception handling filter, which I want to use to generate a problem details response to the caller. So I have to inject a ProblemDetailsFactory, except this is breaking my unit tests. Mocking the factory makes no sense, yet I can't get an instance of it either to use in my test because it's internal.

[captainsafia]

@Carl-Hugo Apologies for the delay in my response to you! This got bumped up in my inbox following @jeremy-allocate's comment.

As a tactical change, we can consider bringing in an API proposal to apply the following:

Another option would be to create a public extension method named AddDefaultProblemDetailsFactory that adds the binding and have AddMvcCore call this new method instead.

IMO, this seems like the most appealing API choice compared to our others one. I recognize the bifurcation of ProblemDetails-related types in the HTTP-layer and the MVC-layer.

Would a new API following the above solve your problems?

[Carl-Hugo]

@Carl-Hugo Apologies for the delay in my response to you! This got bumped up in my inbox following @jeremy-allocate's comment.

As a tactical change, we can consider bringing in an API proposal to apply the following:

Another option would be to create a public extension method named AddDefaultProblemDetailsFactory that adds the binding and have AddMvcCore call this new method instead.

IMO, this seems like the most appealing API choice compared to our others one. I recognize the bifurcation of ProblemDetails-related types in the HTTP-layer and the MVC-layer.

Would a new API following the above solve your problems?

Hi @captainsafia,

That sounds like a quick fix that would allow us to use the default factory outside of MVC (and without registering all MVC "Core" dependencies). So yes, I believe it would solve my problem.

However, the problem details space would remain confusing and scattered and could still benefit from a bit of refactoring. This second larger conversion could probably live better as part of its own GitHub issue, though; what do you think?

Thanks

[captainsafia]

However, the problem details space would remain confusing and scattered and could still benefit from a bit of refactoring. This second larger conversion could probably live better as part of its own GitHub issue, though; what do you think?

We can start it in the GitHub issues for now since that's the most accessible.

I find it a little confusing that the ProblemDetailsFactory and ProblemDetails classes are part of MVC while their reach now goes beyond MVC.

As you noticed here, one of the problems with ProblemDetails is similar to an issue we have with the FromX attributes that are used in MVC and minimal APIs. The APIs associated with the domain existing in MVC namespaces and assemblies, but we want them to be usable outside of MVC as well.

Particularly, in the Microsoft.AspNetCore.Http and Microsoft.AspNetCore.Routing assemblies. Some of the duplication refers to this issue. Another aspect to this is that we did introduce new patterns in minimal APIs that weren't intentionally not implemented in MVC.

A combination of these factors are why we have situations like:

The IProblemDetailsService interface does not directly use the ProblemDetailsFactory, yet leveraging the factory to create an instance of the ProblemDetails class before setting the ProblemDetailsContext would allow consumers to reuse .NET building blocks

The building blocks aren't reusable without type-forwarding or considerable modifications (as in the case of ApiBehaviorOptions) for example. As part of designing this, we opted for choices that would reduce the amount of breaking changes for people using ProblemDetails with MVC while giving us some wiggle room to expose a set of lower level primitives that count eventually compose with both.

It's definitely not an ideal situation, but I think a good path forward is improving the new lower-level primitives and making it relatively painless for people to move between the two patterns in the case that they want to.

I hope this explainer provides some insights into how we got here and the priorities I think we are trying to balance. 😅

[Carl-Hugo]

However, the problem details space would remain confusing and scattered and could still benefit from a bit of refactoring. This second larger conversion could probably live better as part of its own GitHub issue, though; what do you think?

We can start it in the GitHub issues for now since that's the most accessible.

I find it a little confusing that the ProblemDetailsFactory and ProblemDetails classes are part of MVC while their reach now goes beyond MVC.

As you noticed here, one of the problems with ProblemDetails is similar to an issue we have with the FromX attributes that are used in MVC and minimal APIs. The APIs associated with the domain existing in MVC namespaces and assemblies, but we want them to be usable outside of MVC as well.

Particularly, in the Microsoft.AspNetCore.Http and Microsoft.AspNetCore.Routing assemblies. Some of the duplication refers to this issue. Another aspect to this is that we did introduce new patterns in minimal APIs that weren't intentionally not implemented in MVC.

A combination of these factors are why we have situations like:

The IProblemDetailsService interface does not directly use the ProblemDetailsFactory, yet leveraging the factory to create an instance of the ProblemDetails class before setting the ProblemDetailsContext would allow consumers to reuse .NET building blocks

The building blocks aren't reusable without type-forwarding or considerable modifications (as in the case of ApiBehaviorOptions) for example. As part of designing this, we opted for choices that would reduce the amount of breaking changes for people using ProblemDetails with MVC while giving us some wiggle room to expose a set of lower level primitives that count eventually compose with both.

It's definitely not an ideal situation, but I think a good path forward is improving the new lower-level primitives and making it relatively painless for people to move between the two patterns in the case that they want to.

I hope this explainer provides some insights into how we got here and the priorities I think we are trying to balance. 😅

I understand that you want to limit the number of breaking changes and have limited capacity. Thanks for the insights, @captainsafia.

As I mentioned before, adding a new AddDefaultProblemDetailsFactory extension method will do the trick for this issue. The plan to continue building in the Http namespace (lower-level primitives?) sounds like a good compromise indeed. Hopefully, one day, you'll consolidate all of that under a single namespace.

[captainsafia]

I understand that you want to limit the number of breaking changes and have limited capacity. Thanks for the insights, @captainsafia.

Thanks for the understanding here! Hopefully, this will provide some insight to others as well as to how we got here.

As I mentioned before, adding a new AddDefaultProblemDetailsFactory extension method will do the trick for this issue.

Great! Would you be interested in opening a new API proposal for this using the template. I can champion it at our next API review meeting, which will likely occur after the holidays.

[Carl-Hugo]

Great! Would you be interested in opening a new API proposal for this using the template. I can champion it at our next API review meeting, which will likely occur after the holidays.

Certainly, I'll try to do that during the holidays.