Response timeout error from all routes after returning several timeouts from a specific route

[oskouei]

Response timeout error
Return "Response timeout" error from all routes after returning several timeouts from a specific route.

Codes
Set response timeout to 5 sec in config:
RESPONSE_TIMEOUT = 5

Define funcs:

@app.get('test')
async def test_func(request):
    """Timeout func"""
    await asyncio.sleep(10)
    ...
@app.get('test2')
async def test_func2(request):
    """Func without timeout"""
    return response.text('OK')

Make several timeout responses:

for i in range(20):
    resp = requests.get('/test')

Then if I request all other endpoints, it returns "Response timeout" error:
resp = requests.get('/test2')

Environment:

• OS: Ubuntu
• Python: 3.8.2
• Sanic: 20.3.0

[oskouei]

If I add this to the server, no more errors will occur for other routes:

class CustomProtocol(HttpProtocol):
    def response_timeout_callback(self):
        ...
        if self._request_stream_task:
            if not self._request_stream_task.cancelled():
                self._request_stream_task.cancel()
        if self._request_handler_task:
            if not self._request_handler_task.cancelled():
                self._request_handler_task.cancel()

app.create_server(
    ...
    protocol=CustomProtocol
)

Which means I deleted this part of the original code:

self.write_error(...)

But this is not the right solution!

[Tronic]

Thanks for reporting and triaging this. Multiple error responses corrupting HTTP connections was one of the bugs fixed in the yet-to-be-released streaming branch, which you can install by

pip install git+https://github.com/huge-success/sanic.git@streaming

Could you check if that branch solves your problem, so that we can further confirm this and see what needs to be done? Do note that you cannot use custom protocol with the new branch due to the way it restructures everything.

[oskouei]

Thank you for your reply.
I tested this branch and it seemed to work without any problems in this particular case.

[Tronic]

I've asked other core developers for discussion about what to do about it, as directly fixing in master might easily introduce new bugs elsewhere, so care must be taken. I hope that your situation isn't the most pressing sort, because this will in any case take a while to get fixed and released, and probably won't make it into the upcoming 20.6 release.

Technical summary: the current error handling logic may in some situations end up writing a response when one was already sent, corrupting the next request on the same HTTP keep-alive connection. This obviously needs to be avoided (without ever omitting a response in other circumstances). Due to the way the asyncio code, error handling and cancellation are structured, this isn't as trivial as it sounds.

Keeping this report open for tracking the issue.

[ahopkins]

I know it is not the "right solution" either to just hold off and wait. But, I do not want to add any breaking changes between now and December's LTS. The streaming is set to be added for 21.3.

If this turns out to be a larger issue, maybe then we can look to see if there is a temporary patch. In the absence of that, my vote is to wait.