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

Add automatic ping on websockets #947

Merged
merged 4 commits into from Jan 15, 2015

Conversation

Projects
None yet
3 participants
@lultimouomo
Contributor

lultimouomo commented Dec 31, 2014

The ping interval is fixed at 60 seconds.
The best thing would probably be to set it in the server settings, thus also allowing to disable it entirely.
For this, the WebSocket must have access to HTTPServerSettings; the best way would probably be to expose a const reference to HTTPServerRequest.m_settings, which I'm going to do in the next commit of this pull request.

@lultimouomo

This comment has been minimized.

Show comment
Hide comment
@lultimouomo

lultimouomo Dec 31, 2014

Contributor

Apparently older versions of std.conv.to do not support dynamic-to-static array conversion; I've now made the conversion manually to support older toolchains.

Contributor

lultimouomo commented Dec 31, 2014

Apparently older versions of std.conv.to do not support dynamic-to-static array conversion; I've now made the conversion manually to support older toolchains.

@property const(HTTPServerSettings) serverSettings() const
{
return m_settings;
}

This comment has been minimized.

@s-ludwig

s-ludwig Jan 15, 2015

Member

I'd leave this as package for now to avoid any potential necessity for API breakage later. If it turns out that this is useful for external libraries/applications, we can still make it public.

@s-ludwig

s-ludwig Jan 15, 2015

Member

I'd leave this as package for now to avoid any potential necessity for API breakage later. If it turns out that this is useful for external libraries/applications, we can still make it public.

lniccoli added some commits Dec 31, 2014

Add automatic ping on websockets
Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
Add HTTPServerSettings.webSocketPingInterval
It will be made accessible to the websocket via HTTPServerRequest

Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 15, 2015

Member

Looks good to merge, except for the public serverSettings property.

Member

s-ludwig commented Jan 15, 2015

Looks good to merge, except for the public serverSettings property.

lniccoli added some commits Dec 31, 2014

Add HTTPServerRequest.serverSettings
Makes a const reference to HTTPServerSettings accessible to HTTP
delegates.

Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
Read websocket ping interval from server settings
Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
@lultimouomo

This comment has been minimized.

Show comment
Hide comment
@lultimouomo

lultimouomo Jan 15, 2015

Contributor

I made the settings accessor package, and I fixed a bug that came out testing ping timeouts:
the m_pingTimer task was trying to directly close the connection, which was most probably acquired by m_reader. The code now makes m_reader close the connection itself, like the other code paths that lead to the connection being closed.

Contributor

lultimouomo commented Jan 15, 2015

I made the settings accessor package, and I fixed a bug that came out testing ping timeouts:
the m_pingTimer task was trying to directly close the connection, which was most probably acquired by m_reader. The code now makes m_reader close the connection itself, like the other code paths that lead to the connection being closed.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 15, 2015

Member

Great, thanks! I'll merge as soon as the Travis build is ready.

Member

s-ludwig commented Jan 15, 2015

Great, thanks! I'll merge as soon as the Travis build is ready.

s-ludwig added a commit that referenced this pull request Jan 15, 2015

Merge pull request #947 from lultimouomo/pingPong
Add automatic ping on websockets

@s-ludwig s-ludwig merged commit 2303f1c into vibe-d:master Jan 15, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@lultimouomo lultimouomo deleted the lultimouomo:pingPong branch Jan 15, 2015

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