proudmonkey / AutoWrapper

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

Potential improvements to HandleSuccessRequestAsync #5

Closed sondreb closed 5 years ago

sondreb commented 5 years ago

The HandleSuccessRequestAsync method will attempt to perform a check for valid json, convert to JSON string and then serialize to JSON, the response from a call. This happens even if the content being served is HTML. This is highly inefficient and there is probably a much better way to ignore the JSON deserialization when the code discovers the content is not JSON.

image

sondreb commented 5 years ago

As most APIs is located under /api, I decided to copy the AutoWrapper source code and add this extra guard check in the invoke method:

image

This also fixes issue #4

If the API is under another path, that could be configured with AutoWrapperOptions. It is an issue though if you're hosting ApiControllers on root, like the default controller with a new ASP.NET web app where "WeatherForecastController" is accessible through /weatherforecast.

proudmonkey commented 5 years ago

That's interesting! I haven't tested that scenario. Care to share what you've done inside the IsApi() method?

I will revisit the code. I agree that non JSON format should be ignored.

Thank you so much for your feedback!

sondreb commented 5 years ago

IsApi is just a copy of IsSwagger:

        private bool IsApi(HttpContext context)
        {
            return context.Request.Path.StartsWithSegments("/api");
        }
proudmonkey commented 5 years ago

I've released version 1.1.0 and you should now be able to set IsApiOnly in the AutoWrapperOptions. For example:

   app.UseApiResponseAndExceptionWrapper( new AutoWrapperOptions { IsApiOnly = false });

You can also use the WrapWhenApiPathStartsWith option to set your Api path to validate. For example:

app.UseApiResponseAndExceptionWrapper( new AutoWrapperOptions { IsApiOnly = false, WrapWhenApiPathStartsWith = "/myapi" });

I would still prefer to create your APIs in a separate project so you won't mix Angular and ASP.NET Core API routings and configuration.

PS: I've updated the README file to reflect these usage. I also mentioned you for your feedback in this regard. Thank you!