josephwoodward / GlobalExceptionHandlerDotNet

Exception handling as a convention in the ASP.NET Core request pipeline
MIT License
272 stars 32 forks source link

HTTP status code within exception #20

Closed shorbachuk closed 6 years ago

shorbachuk commented 6 years ago

I am using this library, works great for mapping argument exceptions etc.

I have another exception type where I am calling another API. The HTTP status code is within a property in that exception. Is there or can there be a way to handle for this?

josephwoodward commented 6 years ago

At the moment there isn't, but I'd be open to adding an overload to ReturnStatusCode(...)along the lines of:

ReturnStatusCode(Func<Exception, HttpStatusCode>)
shorbachuk commented 6 years ago

Cool, should I work on a PR or is this something you would like to take?

josephwoodward commented 6 years ago

I'm currently doing some work on the API so I can do it as a part of that 👍

shorbachuk commented 6 years ago

Any idea when to expect this enhancement?

josephwoodward commented 6 years ago

Just finishing off the last few bits (I'm working on a major release) so it should be done in the next few days.

josephwoodward commented 6 years ago

Thanks for your patience @shorbachuk,

I've now released a beta package which includes this functionality (it's included within a major version bump I was working on which includes some breaking changes, but they're just method renames). If you'd like to give it a try I'd love to hear how you get on with it.

The new version can be found here https://www.nuget.org/packages/GlobalExceptionHandler/4.0.0-beta1 and the release notes here.

shorbachuk commented 6 years ago

I tried the beta. Agree with all the renames, they make a lot more sense.

I had to do:

x.Map<HttpStatusCodeException>().ToStatusCode(ex => (int)((HttpStatusCodeException)ex).StatusCode);

which is maybe not as clean as I would like, but for sure solves my problem. Thank you

josephwoodward commented 6 years ago

I'd love to be able to pass the generic type passed in at Map<T> all the way down to .ToStatusCode(Func<T, int> x) but unfortunately my GenericFu isn't up to scratch. I'd definitely welcome a PR if you can see some options for making this better?

Another option would also be to add the following overload for those that are using (by choice or necessity) System.Net.HttpStatusCode which would at the very least remove the need to cast the status code to an int?

IHandledFormatters ToStatusCode(Func<Exception, int> statusCodeResolver);
IHandledFormatters ToStatusCode(Func<Exception, HttpStatusCode> statusCodeResolver); // Could add this overload?

The way I see it is it's still in beta so now is the time to make any changes.

josephwoodward commented 6 years ago

25 also adds the overloads to stop people having to cast System.Net.HttpStatusCode to an int, this seams especially helpful in this instance when you can't control the property type within an exception thrown.

josephwoodward commented 6 years ago

@shorbachuk New version with these changes has been uploaded to NuGet. Give it a whirl and let me know what you think.

Alt Text

shorbachuk commented 6 years ago

I just tried it. Its working really great, exactly what I needed.

josephwoodward commented 6 years ago

Great, I'll close this off then. Thanks for the feature suggestion(s)!

shorbachuk commented 5 years ago

Any plans to take this out of beta?

josephwoodward commented 5 years ago

@shorbachuk Yeah, can do it this weekend.

josephwoodward commented 5 years ago

@shorbachuk Just realised I didn't do this! Will do it today, I promise!

Edit: I presume it's all working for you and you've not run into any issues?