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

Automatically reconnect websocket if/when connection is lost #32

Closed
ferchor2003 opened this issue Feb 11, 2021 · 9 comments
Closed

Automatically reconnect websocket if/when connection is lost #32

ferchor2003 opened this issue Feb 11, 2021 · 9 comments

Comments

@ferchor2003
Copy link

The aricpp::Client constructor creates a Websocket connection to get events from the Asterisk server. This is meant to be a long term connection to the server; however, in production environments the websocket can get disconnected. From that point on, no messages will be received from Asterisk.

This issue is related to the following discussion:
#31

@ferchor2003
Copy link
Author

This is the start of a new boost::strand class that makes the aricpp::WebSocket object send a ping to the server every 15 seconds:

#pragma once
#include <boost/asio.hpp>
#include <boost/thread/thread.hpp>
#include <boost/bind/bind.hpp>
#include "aricpp\websocket.h"

class pingAsterisk
{
public:
	pingAsterisk(boost::asio::io_context& io, aricpp::WebSocket& ws) :
		strand_(boost::asio::make_strand(io)),
		timer(io, boost::asio::chrono::seconds(15)),
		ws_(ws)
	{
		timer.async_wait(boost::asio::bind_executor(strand_,
			boost::bind(&pingAsterisk::sendPing, this)));
	}

	~pingAsterisk()
	{}

	void sendPing()
	{
		ws_.startPing();
	}

private:
	boost::asio::strand<boost::asio::io_context::executor_type> strand_;
	boost::asio::steady_timer timer;
	aricpp::WebSocket& ws_;
};

Then we can add a startPing member to the WebSocket:

void startPing()
{
	boost::asio::associated_executor_t<boost::asio::io_service> a = boost::asio::get_associated_executor(ios);
	boost::beast::websocket::ping_data pingWebSocketFrame;
	pingWebSocketFrame.append("aricpp");
	websocket.async_ping(
		pingWebSocketFrame,
		[this](boost::beast::error_code& error_code)
		{
			if (error_code.failed())
			{
				// remedy the situation. Close the current websocket connection and try to restart
				Close();
				std::string req = request;
				ConnectHandler ch = onConnection;
				Connect(req, ch);
			}
	});
}

The application should add the new strand to the ios execution:

  boost::asio::io_context ios;
  aricpp::Client client(ios, host, port, username, password, application);
  pinAsterisk p(ios, client.websocket);
  boost::thread t(boost::bind(&boost::asio::io_context::run, &io));
  client.Connect([&](boost::system::error_code e) {
    ...
  }
  io.run();
  t.join();
  return 0;

This is of course a rough idea, but may be a starting point for the actual development of a class that pings the websocket and reconnects automatically

@daniele77
Copy link
Owner

Thank you very much @ferchor2003 for providing a sketch of a solution, too.
I think we can consider embedding the "ping" inside the aricpp::WebSocket class, maybe with a configurable polling time and optional reconnection. Tell me what you think and then I can try to insert this "feature" in the repository.

@daniele77
Copy link
Owner

I tryied but it's not so simple.

I did this:

class WebSocket
{
public:

...

WebSocket( boost::asio::io_service& _ios, std::string _host, std::string _port ) :
    ios(_ios),
    host(std::move(_host)),
    port(std::move(_port)),
    resolver(ios),
    socket(ios),
    websocket(socket),
    pingTimer(ios, boost::asio::chrono::seconds(10))
{}

...

void Connect( std::string req, ConnectHandler h )
{
    request = std::move(req);
    onConnection = std::move(h);
    resolver.async_resolve(
        boost::asio::ip::tcp::resolver::query{ host, port },
        [this]( const boost::system::error_code& ec, boost::asio::ip::tcp::resolver::iterator i )
        {
            if ( ec ) onConnection( ec );
            else Resolved( i );
        }
    );

    StartPingTimer();
}

...

private:

void Ping()
{
    boost::beast::websocket::ping_data pingWebSocketFrame;
    pingWebSocketFrame.append("aricpp");
    websocket.async_ping(
        pingWebSocketFrame,
        [this](const boost::beast::error_code& error_code)
        {
            if (error_code.failed())
            {
                Close();

                resolver.async_resolve(
                    boost::asio::ip::tcp::resolver::query{ host, port },
                    [this](const boost::system::error_code& ec, boost::asio::ip::tcp::resolver::iterator i)
                    {
                        if ( ec ) onConnection( ec );
                        else Resolved( i );
                    }
                );
            }
        }
    );
}

void PingTimerExpired()
{
    Ping();
    StartPingTimer();
}

void StartPingTimer()
{
    pingTimer.async_wait([this](const boost::system::error_code&){ PingTimerExpired(); });
}

... 

boost::asio::steady_timer pingTimer;
};

unfortunately, when the application (e.g., high_level_dial example w/o the shutdown on disconnection) tries to reconnect, I get a sequence of Connection error: Connection refused errors.

If someone could help with this...

@ferchor2003
Copy link
Author

Try the following to ensure the socket shutsdown:

void Close() noexcept
{
    try
    {
        if ( socket.is_open() )
        {
            if (connected)
            {
                websocket.close(boost::beast::websocket::close_code::normal);
            }
            socket.shutdown(boost::asio::ip::tcp::socket::shutdown_send);
            socket.cancel();
            socket.close();
        }
    }
    catch(const std::exception&)
    {
        // nothing to do
    }
    connected = false;
}

@ferchor2003
Copy link
Author

ferchor2003 commented Feb 12, 2021

I tried your new algorithm and added the call to socket.shutdown() I mentioned. It seems to be working; I can kill the webSocket connection (I'm using an ssh tunnel on port 8080 to my asterisk server, so this is easy to test) and I see it trying to reconnect.
A think it is doing it too fast, so I added a bit of time before each retry:

    void PingTimerExpired()
    {
        // Wait a few seconds before trying to connect back
        boost::this_thread::sleep_for(boost::chrono::seconds{ 5 });
        Ping();
        StartPingTimer();
    }

@daniele77
Copy link
Owner

I just committed a solution:
e3c4fa8
Now there is an optional parameter on Client::Connect that specifies the reconnection period in seconds.
Please, can you tried it and give me feedback?
Thanks

@daniele77
Copy link
Owner

I tried your new algorithm and added the call to socket.shutdown() I mentioned. It seems to be working; I can kill the webSocket connection (I'm using an ssh tunnel on port 8080 to my asterisk server, so this is easy to test) and I see it trying to reconnect.
A think it is doing it too fast, so I added a bit of time before each retry:

    void PingTimerExpired()
    {
        // Wait a few seconds before trying to connect back
        boost::this_thread::sleep_for(boost::chrono::seconds{ 5 });
        Ping();
        StartPingTimer();
    }

The instruction:

boost::this_thread::sleep_for(boost::chrono::seconds{ 5 });

is a blocking call that stops the scheduler (i.e., asio::io_context) for 5 seconds. During those 5 seconds, the application can't receive any messages or commands.
I'm afraid it's not a good idea :-)

@ferchor2003
Copy link
Author

Yeah, the sleep should not be on every timer call. I have moved it to the Ping function, to wait a few seconds in the case of an error before trying to reconnect again.
Also, socket.shutdown() should only be sent if we are connected. So:

        if ( socket.is_open() )
        {
            if (connected)
            {
                socket.shutdown(boost::asio::ip::tcp::socket::shutdown_send);
                websocket.close(boost::beast::websocket::close_code::normal);
            }
            

@daniele77
Copy link
Owner

@ferchor2003 I tried the code you're suggesting but, often on reconnection the following assertion fails:

high_level_dial: /home/daniele/lib/boost_1_74_0/install/gcc-8.3.0/include/boost/beast/websocket/impl/stream_impl.hpp:192: 
void boost::beast::websocket::stream< <template-parameter-1-1>, <anonymous> >::impl_type::reset() [with NextLayer = 
boost::asio::basic_stream_socket<boost::asio::ip::tcp>&; bool deflateSupported = true]: Assertion `status_ != status::open' failed.

so I kept the socket.shutdown() outside of the if statement.

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

No branches or pull requests

2 participants