Fix for #147 #185
Fix for #147 #185
Conversation
|
|
|
@Tratcher I would appreciate you giving this a look. Seems reasonable to me though it adds a few more overloads to the public surface. Any concerns? |
| catch (Exception exception) | ||
| { | ||
| if (_rethrowExceptions) | ||
| throw; |
There was a problem hiding this comment.
style: include the braces please.
| { | ||
| public class CorsMessageHandlerTest | ||
| { | ||
| private class PassthroughExceptionHandler : IExceptionHandler |
There was a problem hiding this comment.
I feels like it's not the best place for this class, however i have no idea where it should be placed(
Any suggestions would be appreciated
There was a problem hiding this comment.
Bottom of this class is fine.
| { | ||
| HttpConfiguration config = new HttpConfiguration(); | ||
| config.Routes.MapHttpRoute("default", "{controller}"); | ||
| config.Services.Replace(typeof(IExceptionHandler), new PassthroughExceptionHandler()); |
There was a problem hiding this comment.
This is needed to reproduce the exact scenario of original issue, otherwise default implementation of IExceptionHandler maps Exceptions to just 500 status code
|
I added tests and braces. |
| { | ||
| public class CorsMessageHandlerTest | ||
| { | ||
| private class PassthroughExceptionHandler : IExceptionHandler |
There was a problem hiding this comment.
Bottom of this class is fine.
| public static void EnableCors(this HttpConfiguration httpConfiguration) | ||
| { | ||
| EnableCors(httpConfiguration, null); | ||
| EnableCors(httpConfiguration, null, false); |
There was a problem hiding this comment.
FYI we've got a meeting scheduled next Tuesday to decide whether default should be true i.e. whether we should break compatibility to ensure IExceptionHandler sees what it should. Will let you know…
There was a problem hiding this comment.
Decision was to avoid breaking changes and stick with the approach you've implemented here.
4 participants
Adresses #147