search

valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.91k stars 1.76k forks source link

Allow connection close for custom streams #1603

Closed becheran closed 1 year ago

becheran commented 1 year ago

We do use fasthttp for our project as a proxy between an uplink connection and a target. We discovered the following scenario where we run into an issue that we can't solve without a minor change to the fasthttp client.

The scenario that we have is the following:

From the upsteam connection we get a request which is replied with a steam of data from the target. All works fine using fasthttp and the customStream body. We simply copy from the target reader to the upstream writer. Now the upstream suddenly closes the TCP connection and we get an error while trying to copy our data from the target.

Now we definitelly want to close the still open connection to the target. The only thing we can do from within the handler is set fastRequest.SetConnectionClose() and fastResponse.SetConnectionClose(). Though, the TCP connection will not be closed since it is too late when we set those values. The TCP connection is released and eventually re-used by the next upstream connection which would then result into an unexpected error.

If the closeConn check is done again in the newCloseReader wrapper function we could set the ConnectionClose() property from the outside and the connection will be closed once we release our request handler.

becheran commented 1 year ago

Interesting. Don't have access to a computer right now. Will have a deeper look next week.

becheran commented 1 year ago

@erikdubbelboer I did fix the TestAllocationClient but for the next run the TestStreamBodyRequestContentLength test failed. Though, I can't reproduce this locally and am also not sure how I could have possibly broken this? It is in the server side which I did not touch with the PR. Is this some test that is known to be flaky?

erikdubbelboer commented 1 year ago

Thanks! Some tests can be flaky, especially on windows. It all looks good now.