search

ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.32k stars 1.63k forks source link

ResponderMiddleware will never set a 500 response code #66

Closed MarcDenman closed 7 years ago

MarcDenman commented 7 years ago

ResponderMiddleware.cs looks as if it can set a 500 to be returned, however, the implementation of ErrorsToHttpStatusCodeMapper creates a new OkResponse and does not pass the list of errors through.

As such, when ResponderMiddleware calls IsError false will always be returned.

I am 85% sure what I have said above is correct, as I am still learning the codebase, happy to PR though if I can be pointed in the correct direction.

TomPallister commented 7 years ago

@MarcDenman you are correct, I would change the ErrorsToHttpStatusCodeMapper to not return with a wrapped Response value. Then you dont need the check in the middleware.

Would be great if you could make that change otherwise just leave it here and ill get to it. Good spot.

Really grateful for the assistance. I know from experience getting a second pair of eyes on my code will pick up these kind of issues so much appreciated.

MarcDenman commented 7 years ago

Excellent, not a problem, happy to help out :)

I have made a PR with the changes and fixed up the unit tests

TomPallister commented 7 years ago

@MarcDenman merged PR closing issue!