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 doHandleWebsocket function #946

Merged
merged 2 commits into from Jan 15, 2015

Conversation

Projects
None yet
5 participants
@lultimouomo
Contributor

lultimouomo commented Dec 29, 2014

This allows establishing a websocket connection directly in an
HTTPServerRequestDelegate, which is useful if one needs to make some
checks on the request (and possibly send an http response with an error
code) before opening the websocket.

Signed-off-by: Luca Niccoli l.niccoli@awtech.it

Add doHandleWebsocket function
This allows establishing a websocket connection directly in an
HTTPServerRequestDelegate, which is useful if one needs to make some
checks on the request (and possibly send an http response with an error
code) before opening the websocket.

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

@lultimouomo lultimouomo reopened this Dec 31, 2014

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 15, 2015

Member

Thanks, this change makes a lot of sense. I'm just still a little unsure about the best name. Usually I'd simply use an additional overload of handleWebSockets (analogous to handleBasicAuth), but of course the plural form doesn't really fit here. The doXYZ style generally shouldn't appear in the public API, ideally everything should be a simple verb or verbNoun combination. If no better alternative comes up, I'd suggest simply handleWebSocket.

Member

s-ludwig commented Jan 15, 2015

Thanks, this change makes a lot of sense. I'm just still a little unsure about the best name. Usually I'd simply use an additional overload of handleWebSockets (analogous to handleBasicAuth), but of course the plural form doesn't really fit here. The doXYZ style generally shouldn't appear in the public API, ideally everything should be a simple verb or verbNoun combination. If no better alternative comes up, I'd suggest simply handleWebSocket.

Clean up doHandleWebsocket
Do not declare the WebSocket a scoped variable, it's dangerous and does
not seem to be useful anyway, since WebSocket does not declare a
destructor.
Do not simulate socket closing if the connection delegate throws an
Error, the recommended action is terminating the application anyway.

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

handleWebSocket seems fine to me.

Contributor

lultimouomo commented Jan 15, 2015

handleWebSocket seems fine to me.

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

@s-ludwig s-ludwig merged commit 5abe244 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:doHandleWebsocket branch Jan 15, 2015

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