ProblemDetails "Instance" property is not being set about middleware HOT 13 CLOSED

The reason it isn't set is because it's hard to follow the RFC in generic exception or status code handling:

instance (string) - A URI reference that identifies the specific occurrence of the problem. It may or may not yield further information if dereferenced.

If you have an application-specifc way to set the instance property, you can derive from the ProblemDetailsFactory class and register it in the container before calling AddProblemDetails. That should allow you to customize how the problem responses are generated however you like.

from middleware.

In section 3 of that RFC, there is an example where instance looks like this:
"instance": "/account/12345/msgs/abc",
And in section 4, there is this text:
"In summary: an instance URI will always identify a specific occurrence of a problem"

So although the description of the instance member (section 3.1) is a bit too vague, from the examples it looks like instance should contain the URI that initiated the request, or the URI where the problem occurred.
When you say it's hard to follow the RFC in generic exception or status code handling, do you mean it is due to technical reasons? Maybe the context is not known at the point?

from middleware.

from the examples it looks like instance should contain the URI that initiated the request, or the URI where the problem occurred.

Maybe, maybe not. I don't think it's crystal clear by the path in the sample that it's referring to the endpoint that restulted in the error or whether it's referring to a specific instance of the error.

I've always read the instance property as something referring to the exact instance of an error, i.e. a request ID or something. The path of the request could yield any number of errors; 400, 401, 403, 404, 422, 500, etc. I don't necessarily think that the path itself will be enough to "identify a specific occurrance of a problem".

When you say it's hard to follow the RFC in generic exception or status code handling, do you mean it is due to technical reasons? Maybe the context is not known at the point?

Yeah, being a generic error handler middleware, there's only so much you know about the world you live in. For everything else, you rely on the developer(s) to fill in the missing pieces through configuration and extensibility points. I.e. maybe the API has a set of docs pages for different errors. The problem responses should probably link to those.

from middleware.

Hmm, I see your point. Yeah, the specification is not clear enough, hence difficult to assume something for all. And the idea of using the instance to carry an error ID is good, although .NET already includes the request ID as traceId in the response. Maybe that is enough :)
I'm going to try creating a custom, inherited ProblemDetailsFactory just to see how it goes.
Thanks!

from middleware.

Btw, I am trying to register my custom ProblemDetailsFactory before AddProblemDetails() but I keep getting the default (from your middleware) one. If I register it after AddProblemDetails(), then it works. This is the code:

Program.cs

builder.Services.AddProblemDetails(true);
builder.Services.AddSingleton<Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory, MyCustomProblemDetailsFactory>();
//or builder.Services.Replace(new ServiceDescriptor(typeof(Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory), typeof(MyCustomProblemDetailsFactory), ServiceLifetime.Singleton));

MyCustomProblemDetailsFactory.cs

using HPM = Hellang.Middleware.ProblemDetails;
public class MyCustomProblemDetailsFactory : HPM.ProblemDetailsFactory
{
        public MyCustomProblemDetailsFactory(IOptions<ProblemDetailsOptions> options, ILogger<ProblemDetailsFactory> logger, IHostEnvironment environment) : base(options, logger, environment)
        {
        }

        public override ProblemDetails? CreateExceptionProblemDetails(HttpContext context, Exception error)
        {
            var pd = base.CreateExceptionProblemDetails(context, error);
            if (pd != null)
                pd.Instance = context.Request.Path;

            return pd;
        }

        // ... other overrides omitted for simplicity, but they are pretty similar to the one above.
}

In my controller I'm not receiving anything in the constructor, the ControllerBase.ProblemDetailsFactory property is being correctly resolved :)

Does that look OK from your view?

One last question: by creating my own factory, it is still not being used when there is an unhandled exception being handled by your middleware. Is that expected? Any way to use it there as well or override that behavior to being able to fill the Instance property?

Thanks

from middleware.

Yeah, you need to derive from Hellang.Middleware.ProblemDetails.ProblemDetailsFactory (which in turn inherits MVC's factory) in order to properly override it.

from middleware.

Yep, that's what I did :)

using HPM = Hellang.Middleware.ProblemDetails;
public class MyCustomProblemDetailsFactory : HPM.ProblemDetailsFactory

from middleware.

Ah, but you didn't register it as that;

builder.Services.AddSingleton<Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory, MyCustomProblemDetailsFactory>();

from middleware.

Oh, should I actually register my instance for the type of Hellang.Middleware.ProblemDetails.ProblemDetailsFactory rather than for Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory?

builder.Services.AddSingleton<Hellang.Middleware.ProblemDetails.ProblemDetailsFactory, MyCustomProblemDetailsFactory>();

Like this?
I wonder because with the other way around, it seems to work as well. But maybe I'm missing some details :)

from middleware.

Yes. The custom factory will be used for cases involving MVC, but the middleware will still use the default factory, hence:

by creating my own factory, it is still not being used when there is an unhandled exception being handled by your middleware. Is that expected?

When you register as Hellang.Middleware.ProblemDetails.ProblemDetailsFactory and then call AddProblemDetailsConventions when adding MVC, it'll also override the built-in registration for Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory:

from middleware.

I don't know why, but I tried both and both seem to work. Am I missing something?

// Both ways work
builder.Services.AddSingleton<Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory, MyCustomProblemDetailsFactory>();
builder.Services.AddSingleton<Hellang.Middleware.ProblemDetails.ProblemDetailsFactory, MyCustomProblemDetailsFactory>(); 

This is the complete code:

Program.cs

public static void Main(string[] args)
{
    var builder = WebApplication.CreateBuilder(args);
    builder.Services.AddControllers();

    builder.Services.AddProblemDetails(
        options =>
        {
            options.ValidationProblemStatusCode = StatusCodes.Status400BadRequest;

            options.MapToStatusCode<ArgumentException>(StatusCodes.Status400BadRequest);
            options.MapToStatusCode<NotImplementedException>(StatusCodes.Status501NotImplemented);
            options.MapToStatusCode<NotSupportedException>(StatusCodes.Status405MethodNotAllowed);
            options.MapToStatusCode<Exception>(StatusCodes.Status500InternalServerError);
        }
    );

    services.AddProblemDetailsConventions();

    // Replace default ProblemDetails factory with my own custom factory...
    // Both lines below seem to work and give the same result when throwing new Exception(), no mather whether 
    // one has called options.MapToStatusCode<Exception>(); or not.

    //builder.Services.AddSingleton<Microsoft.AspNetCore.Mvc.Infrastructure.ProblemDetailsFactory, MyCustomProblemDetailsFactory>();
    builder.Services.AddSingleton<Hellang.Middleware.ProblemDetails.ProblemDetailsFactory, MyCustomProblemDetailsFactory>();

    builder.Services.AddEndpointsApiExplorer();
    builder.Services.AddSwaggerGen();

    var app = builder.Build();

    // Configure the HTTP request pipeline.
    if (app.Environment.IsDevelopment())
    {
        app.UseSwagger();
        app.UseSwaggerUI();
    }

    app.UseProblemDetails();
    app.UseAuthorization();
    app.MapControllers();
    app.Run();
}

MyCustomProblemDetailsFactory.cs

using HPM = Hellang.Middleware.ProblemDetails;
public class MyCustomProblemDetailsFactory : HPM.ProblemDetailsFactory
{
        public MyCustomProblemDetailsFactory(IOptions<ProblemDetailsOptions> options, ILogger<ProblemDetailsFactory> logger, IHostEnvironment environment) : base(options, logger, environment)
        {
        }

        public override ProblemDetails? CreateExceptionProblemDetails(HttpContext context, Exception error)
        {
            var pd = base.CreateExceptionProblemDetails(context, error);
            if (pd != null)
                pd.Instance = context.Request.Path;

            return pd;
        }

        // ... other overrides omitted for simplicity, but they are pretty similar to the one above (I've overridden all)
}

WeatherForecastController.cs (I'm using for testing different scenarios)

[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
{
    public WeatherForecastController()
    {
    }

    [HttpGet("get-weather-by-city-code")]
    public ActionResult<IEnumerable<WeatherForecast>> GetWatherByCityCode(int cityCode)
    {
        if (cityCode > 900)
            return NotFound(ProblemDetailsFactory.CreateProblemDetails(
                HttpContext,
                StatusCodes.Status404NotFound,
                "Not found",
                "https://mytype.url/404",
                "Not found!",
                Request.Path));

        if (cityCode > 800)
            return BadRequest(
                ProblemDetailsFactory.CreateValidationProblemDetails(
                    HttpContext, 
                    new ModelStateDictionary()));

        if (cityCode > 700)
            throw new AuthenticationException("Authentication exception");

        if (cityCode > 600)
            throw new Exception("Generic exception");

        if (cityCode > 500)
            throw new NotImplementedException();

        if (cityCode > 100)
            return BadRequest(
                ProblemDetailsFactory.CreateProblemDetails(
                    HttpContext,
                    StatusCodes.Status400BadRequest, 
                    "Validation issues.", 
                    "https://mytype.url/400", 
                    "City code cannot be greater than 100", 
                    Request.Path));

        if (cityCode > 50)
            throw new FormatException();

        if (cityCode < 30)
            throw new ArgumentException("< 30");

        return Enumerable.Range(1, 5).Select(index => new WeatherForecast
        {
            Date = DateTime.Now.AddDays(index),
            TemperatureC = Random.Shared.Next(-20, 55)
        })
        .ToArray();
    }
}

If value 703 is passed for cityCode paramenter, my custom factory gets called and it creates a PD with filled Instance property.

from middleware.

I'm pretty sure the factory in the middleware constructor refers to Hellang.Middleware.ProblemDetails.ProblemDetailsFactory (it's too bad I didn't choose another name to differentiate it):

Unless you register your own custom factory type in the container prior to calling AddProblemDetails, it will register and use the default implementation:

Once you call AddProblemDetailsConventions, that should override MVC's default factory by forwarding to this library's default factory:

Did you have both lines at the same time or did you try with each individuallly and they both call your custom factory? Anyway, as long as it works, I don't see a huge problem 😅

from middleware.

I'm pretty sure the factory in the middleware constructor refers to Hellang.Middleware.ProblemDetails.ProblemDetailsFactory

Looking at the code, I'm trying to understand which factory is being used, since there is a using Microsoft.AspNetCore.Mvc.Infrastructure; in the same file 😅

Did you have both lines at the same time or did you try with each individuallly and they both call your custom factory?

I use one line each time, not both at the same time, and yes they both end up calling my custom factory 🤨

Anyway, as long as it works, I don't see a huge problem

Great, that's good enough for me as well 😁
Thanks a lot!

from middleware.