#749 Bad error handling for `IOException` while reading incoming request body

ThreeMammals / Ocelot

.NET API Gateway

https://www.nuget.org/packages/Ocelot

MIT License

8.32k stars 1.63k forks source link

#749 Bad error handling for `IOException` while reading incoming request body #1769

Closed ggnaegi closed 6 months ago

ggnaegi commented 10 months ago

Fixes #749

The error handling in ocelot is hiding the 413 payload too large exception. As @raman-m pointed out, a deeper error handling review is needed. Here, I have implemented a quick fix to this issue and I would recommend opening a new issue for the error handling review.

Proposed Changes

Handling BadHttpRequestException and if status code == 413, then returning a PayloadTooLargeError
Adding two acceptance tests for kestrel and HttpSys instances

ggnaegi commented 10 months ago

@raman-m I just added a skippable fact, since httpsys is compatible with Windows platform only.

raman-m commented 10 months ago

Please, resolve conflicts!

ggnaegi commented 9 months ago

@raman-m you can review it, code is not great, but it's working. I propose reviewing the exception handling later...

raman-m commented 9 months ago

@ggnaegi commented on Dec 14

Thanks for update! Not urgent. Not planned. I'll review after the current Nov'23 release... You know we're focusing on it now...

ggnaegi commented 9 months ago

@ggnaegi commented on Dec 14

Thanks for update! Not urgent. Not planned. I'll review after the current Nov'23 release... You know we're focusing on it now..

@raman-m but we discussed this... remember, but ok for later.

raman-m commented 7 months ago

@ggnaegi Please resolve conflicts and rebase onto develop! I need to decide on release version to include it into. What's readiness of the PR? Will you develop more? If it is highly ready then we can add the PR to Jan'24 release.

ggnaegi commented 7 months ago

@raman-m Yes, it can be part of Jan 24 Release

raman-m commented 6 months ago

@ggnaegi Please resolve conflicts!