khellang / Middleware

Various ASP.NET Core middleware
MIT License
811 stars 112 forks source link

Problem with exceptions hierarchy and problem details mapping #170

Closed ImoutoChan closed 2 years ago

ImoutoChan commented 2 years ago

Consider the following code

public class MyAppException : Exception {}

public class UserMyAppException : MyAppException 
{ 
    public Guid UserId { get; set; }
}

public class MyAppProblemDetails : ProblemDetails 
{ 
    public MyAppProblemDetails(MyAppException exception) 
    {
        Status = StatusCodes.Status400BadRequest;
        Type = exception.GetType().Name;
        Title = "Unable to perform the action";
        Detail = exception.Message;
    }
}

public class UserMyAppProblemDetails : ProblemDetails  
{ 
    public UserMyAppProblemDetails(UserMyAppException exception) 
    {
        Status = StatusCodes.Status400BadRequest;
        Type = exception.GetType().Name;
        Title = "Unable to perform the action with user";
        Detail = exception.Message;
        UserId = exception.UserId
    }

    public Guid UserId { get; set; }
}

I have a basic exception type for my service and some specific exception with additional properties.

I also want to map these exceptions to described ProblemDetails models. Specific problem details models have additional properties (UserId in this example).

Then consider the following mapping configuration I have

services.AddProblemDetails(o => {
      o.Map<MyAppException>(e => new MyAppProblemDetails(e));
      o.Map<UserMyAppException>(e => new UserMyAppProblemDetails(e));
});

Now, the problem is that configuration doesn't work. It will select the first mapper every time, instead of going with the most specific one. So if I have a mapper for Exception it will always use it for all exceptions and ignore other mappings.

Is there any way to bypass that? (changing the order of mapping registration is not a solution, it's unmanageable when you have a lot of exceptions and models).

Maybe we can fix this somehow

khellang commented 2 years ago

Hello @ImoutoChan! 👋

Have you tried switching the registration order? 🤔

ImoutoChan commented 2 years ago

Hi!

As I already mention, switching the registration order will help in this case, but that isn't possible in other situations.

Firstly, there could be many exceptions and correspondent problem details models, and manually sorting them every time is not a good option. Secondly, there could be solutions to register them automatically, and it's not possible to sort registration in that case.

The best solution would be to find the closest exception type in the hierarchy. The second best would be to match exact exception type first and then try everything else.

khellang commented 2 years ago

Secondly, there could be solutions to register them automatically, and it's not possible to sort registration in that case.

What are these solutions? Why is it not possible to sort them? And what makes you think Scrutor can sort them internally if this solution can't do it?

The best solution would be to find the closest exception type in the hierarchy. The second best would be to match exact exception type first and then try everything else.

I agree, but it's a breaking change to make the configuration order independent so it probably won't happen for a while (if ever).

ImoutoChan commented 2 years ago

Ok, you have a point, whatever your library can do inside to determine which mapping it should use, I could just use the same mechanic and change the order of registrations accordingly.

it probably won't happen for a while

That's sad

khellang commented 2 years ago

That's sad

Breaking 4 164 313 people happily using ordered registrations is even sader unfortunately 😞