proudmonkey / AutoWrapper

A simple, yet customizable global exception handler and Http response wrapper for ASP.NET Core APIs.
MIT License
679 stars 82 forks source link

[AutoWrapIgnore] doesn't work with Error Response #83

Open MykolaKlymenko opened 3 years ago

MykolaKlymenko commented 3 years ago

When action method is under [AutoWrapIgnore] attribute, I still get AutoWrapper response structure for Error Response.

image

image

proudmonkey commented 3 years ago

That's the behavior and expected. AutoWrapIgnore will only work non exception request.

AdamSchueller commented 3 years ago

@proudmonkey what is the recommended way to add AutoWrapper to a project in a way that doesn't introduce breaking changes to existing APIs -- including questionable error responses, since clients are begrudgingly accustomed to them?

The behavior seems to have changed between v3.0.0 (which supports netcoreapp2.X) and v4+ (which only support netcoreapp3+), when exception filters are in use:

image

image

The later behavior allows us to add AutpWrapper to the project, then sprinkle ignores on on the pre-existing controllers, for a nice "opt out" behavior.

Not sure if this was intentional or a change in ASP.NET Core 3+? We're planning to update everything to .NET Core 3.1 eventually, but we still have an active 2.X fork that we'd like to use AutoWrapper in. We were looking at using routes to segregate the behavior (e.g. /v1 vs /v2), but curious if there's a better way.

proudmonkey commented 3 years ago

Not sure I'm following. I know there were a few changes in terms of implementation after v 3.0.0 but the behavior of [AutoWrapIgnore] should stay the same; and that is to ignore successful requests be "wrapped". For exceptions and if you are using AutoWrapper, it should still wrap the error response.

The latest version that supports .NET Core 2.x is AutoWrapper 3.0.0. This version doesn't support new features that has been added in AutoWrapper 4.x.x version.

MykolaKlymenko commented 3 years ago

We need to support the old API without AutoWrapper and the new one with Autowrapper. Old endpoints should works as before (in both cases: success and error response). Approach with applying middleware based on the route will not be suitable for us. We need some approach for ignoring AutoWrapp on error response too. [AutoWrapIgnore] works well only with a Success response.

proudmonkey commented 3 years ago

@MykolaKlymenko the only way that you could try at the moment to be able to combine your old and new endpoints is to group your new endpoints into a new url prefix and use WrapWhenApiPathStartsWith option to only activate AutoWrapper for new endpoints.

app.UseApiResponseAndExceptionWrapper( new AutoWrapperOptions { 
          WrapWhenApiPathStartsWith = "/v2" 
});