Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Websockets connections receiving "Response Timeout" #969

Closed
furious-luke opened this issue Oct 11, 2017 · 10 comments
Closed

Websockets connections receiving "Response Timeout" #969

furious-luke opened this issue Oct 11, 2017 · 10 comments

Comments

@furious-luke
Copy link
Contributor

Hi there, I'm finding that websockets connections are getting cancelled from the server as a result of a 60 second timeout. The error looks like this:

[2017-10-11 14:43:54 +1100] [11644] [ERROR] Traceback (most recent call last):
  File ".../sanic/sanic/server.py", line 178, in response_timeout_callback
    raise ServiceUnavailable('Response Timeout')
sanic.exceptions.ServiceUnavailable: Response Timeout

This issue can be reproduced with a simple server like this:

import time
from sanic import Sanic
from sanic.response import json

app = Sanic()

@app.websocket('/')
async def test(request, ws):
    time.sleep(120)
    ws.send('hello')

if __name__ == "__main__":
    app.run(host="0.0.0.0", port=8000)

And a client like this:

import asyncio
import websockets

async def go():
    async with websockets.connect('ws://localhost:8000') as ws:
        await ws.recv()

loop = asyncio.get_event_loop()
loop.run_until_complete(go())

Is there something obvious I'm doing wrong?

Thanks for your help!

@CharAct3
Copy link

CharAct3 commented Oct 11, 2017

time.sleep(120) blocks the server, use:

import asyncio


@app.websocket('/')
async def test(request, ws):
    await asyncio.sleep(120)
    await ws.send('hello')

@furious-luke
Copy link
Contributor Author

Hey @CharAct3, that's a fair point about blocking the server, however I don't think it's related to the websockets getting clobbered. I've altered the server code to use await asyncio.sleep(120) and the websockets are still getting killed by a timeout with the same error.

@CharAct3
Copy link

Hey @furious-luke , you're right.
I was using sanic with version 0.6.0, so I did't meet this problem.
It's caused by HttpProtocol.response_timeout.

@furious-luke
Copy link
Contributor Author

It looks like it shouldn't be affecting websockets, at least by the code in sanic/websocket.py:

    def connection_timeout(self):
        # timeouts make no sense for websocket routes
        if self.websocket is None:
            super().connection_timeout()

Is it possible this method is being ignored for some reason?

@CharAct3
Copy link

@furious-luke Yes, this method is being ignored.

WebsocketProtocol inherits HttpProtocol.

And HttpProtocol.connection_timeout has been splited into three methods now:
request_timeout_callback
response_timeout_callback
keep_alive_timeout_callback

ref #939

@furious-luke
Copy link
Contributor Author

Ah, good to know.

It looks like I can just rename that method from connection_timeout to response_timeout_callback to resolve this issue on my end, does that sound right?

@CharAct3
Copy link

@furious-luke Yes, here is a monkey patch.

from sanic.websocket import WebSocketProtocol

...

class CustomWebSocketProtocol(WebSocketProtocol):

    def request_timeout_callback(self):
        if self.websocket is None:
            super().request_timeout_callback()

    def response_timeout_callback(self):
        if self.websocket is None:
            super().response_timeout_callback()

app.run(protocol=CustomWebSocketProtocol)

@furious-luke
Copy link
Contributor Author

Cool, thanks for your help!

@ashleysommer
Copy link
Member

As the author of #939, I apologise. I did not test the changes on a websocket connection, and I didn't realize the Websocket protocol inherits from the HTTP Protocol.

I have created a PR with the fix here: #976

@furious-luke
Copy link
Contributor Author

No need for apologies! Thanks for responding and making a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants