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

Network adapter going down does not disconnect the websockets #827

Closed
Insoleet opened this issue Mar 13, 2016 · 11 comments
Closed

Network adapter going down does not disconnect the websockets #827

Insoleet opened this issue Mar 13, 2016 · 11 comments

Comments

@Insoleet
Copy link
Contributor

Long story short

I'm working on a software which uses aiohttp websockets to connect to multiple network nodes. There is a bug with aiohttp and its websockets. When the users are disconnecting from wifi, or set their computers on standby and waking their computers later on, the software does not detect the disconnection.

Expected behaviour

When the network adapter is disconnected, the websockets should raise an exception and close themselves.

Actual behaviour

The websockets do not close.

Steps to reproduce

1- When connected to the internet, run the following code :

import aiohttp
import asyncio

async def connect():
    try:
        async with aiohttp.ws_connect('ws://metab.ucoin.io:9201/ws/block') as the_ws:
            async for msg in the_ws:
                if msg.tp == aiohttp.MsgType.text:
                    print(msg.data)
                elif msg.tp == aiohttp.MsgType.closed:
                    print("Closed")
                    break
                elif msg.tp == aiohttp.MsgType.error:
                    print("Error")
                    break
    except Exception as e:
        print(str(e))
    finally:
        print("Disconnected")

loop = asyncio.get_event_loop()
loop.run_until_complete(connect())
print("Finished")

2- It will print json data from an ucoin blockchain. Disconnect your internet
3- It never prints the exception, neither "Disconnected" nor "Finished" messages.

Your environment

I'm using python3 with archlinux 64 bits.

@asvetlov asvetlov added this to the 0.22 milestone Mar 15, 2016
@asvetlov
Copy link
Member

Thanks for report.

@asvetlov
Copy link
Member

The patch is welcome :)

@Insoleet
Copy link
Contributor Author

So I looked at it all this afternoon, but I'm not sure how to fix this bug.

This is where I'm stuck : I'm not sure what to do now. The asyncio documentation describes that

BaseProtocol.connection_lost(exc)
Called when the connection is lost or closed.
The argument is either an exception object or None. The latter means a regular EOF is received, or the connection was aborted or closed by this side of the connection.

So I'm not sure why it is never called. Any help would be greatly appreciated !

@fafhrd91
Copy link
Member

If connection_lost is not being called aiohttp can not do anything. But that might be buggy device driver

@fafhrd91
Copy link
Member

also you can put some print statements into asyncio event loop and see if asyncio received info from kernel for specific socket.

@Insoleet
Copy link
Contributor Author

Thanks for the info. I'm just wondering, can you reproduce the error I described in the first post of this issue ? Just to check if it's a bug happening on some platforms only.

Where should I put trace logging in asyncio loop ?

@Insoleet
Copy link
Contributor Author

After reading this : http://superuser.com/questions/1021988/connection-remains-flagged-as-established-even-if-host-is-unconnected
I supposed that enabling autoping would help detecting the lost connection. But autoping does not send ping automatically. Is this a bug or a feature ?

@Insoleet
Copy link
Contributor Author

Ok I found a way to detect disconnections by using a heartbeat mecanism :

    async def heartbeat(ws):
        """

        :param asyncio.WebsocketClientResponse ws: the ws to do heartbeat with
        :return:
        """
        future_pong = None

        def pong_callback():
            if future_pong:
                future_pong.set_result(True)

        async def ping_loop():
            nonlocal future_pong
            future_pong = asyncio.Future()
            missed_heartbeats = 0
            while not ws.closed:
                await asyncio.sleep(15)
                if not ws.closed:
                    try:
                        future_pong = asyncio.Future()
                        ws.ping()
                        await asyncio.wait_for(future_pong, 15)
                        missed_heartbeats = 0
                    except asyncio.TimeoutError:
                        missed_heartbeats += 1
                    finally:
                        if missed_heartbeats > 3:
                            await ws.close()

        asyncio.ensure_future(ping_loop())
        return pong_callback

I use it this way :

import aiohttp
import asyncio

async def connect():
    try:
        async with aiohttp.ws_connect('ws://metab.ucoin.io:9201/ws/block') as the_ws:
            heartbeat_cb = heartbeat(the_ws)
            async for msg in the_ws:
                if msg.tp == aiohttp.MsgType.text:
                    print(msg.data)
                elif msg.tp == aiohttp.MsgType.closed:
                    print("Closed")
                    break
                elif msg.tp == aiohttp.MsgType.error:
                    print("Error")
                    break
                elif msg.tp == aiohttp.MsgType.pong:
                    heartbeat_cb()
    except Exception as e:
        print(str(e))
    finally:
        print("Disconnected")

loop = asyncio.get_event_loop()
loop.run_until_complete(connect())
print("Finished")

I suppose it could be used for the following issue : #777

What do you think ?

@asvetlov
Copy link
Member

Heartbeat should be implemented by aiohttp.
Unfortunately not all servers support ping/pong game :(

@asvetlov asvetlov modified the milestones: 1.1, 1.0 Sep 16, 2016
@asvetlov asvetlov modified the milestones: 1.2, 1.1 Dec 7, 2016
@fafhrd91
Copy link
Member

fafhrd91 commented Feb 1, 2017

added auto ping_interval parameter for client and server, should solve this problem.

@fafhrd91 fafhrd91 closed this as completed Feb 1, 2017
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants