scottoffen / grapevine-legacy

Embedding A REST Server In Your Application Should Be Simple
http://scottoffen.github.io/grapevine-legacy/
Apache License 2.0
209 stars 51 forks source link

Fix for issue 188 causes the SendsInternalServerErrorResponseWhenExceptionThrown test to fail #205

Closed mattdefoor closed 5 years ago

mattdefoor commented 5 years ago

The fix for #188 causes the SendsInternalServerErrorResponseWhenExceptionThrown test to fail.

The relevant diff is here: https://github.com/sukona/Grapevine/commit/4003de2dcad909925c6bd4e803b4b4b495cc681f#diff-989c985080ab75fc3eac435c40f43432

Notice the logic in the ternary operation of var status at line 557:

var status = (context.Response.StatusCode == HttpStatusCode.Ok) ? HttpStatusCode.InternalServerError : context.Response.StatusCode;

I believe that it should be the following:

var status = (context.Response.StatusCode != HttpStatusCode.Ok) ? HttpStatusCode.InternalServerError : context.Response.StatusCode;

Otherwise, status is set to 0 (the value of context.Response.StatusCode) and the test fails because it does not receive the proper HttpStatusCode.InternalServerError.

Besides, if I'm not mistaken, when an exception is countered, context.Response.StatusCode would never be HttpStatusCode.Ok and if it were, should it return HttpStatusCode.InternalServerError?

scottoffen commented 5 years ago

Thanks!