Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

docs(KEEP_ALIVE): updated protocol documentation to explain the correct workflow between connection_init, connection_ack and connection_keep_alive messages #279

Merged
merged 1 commit into from Oct 7, 2017

Conversation

mistic
Copy link
Contributor

@mistic mistic commented Sep 22, 2017

This PR fixes a missing information on the protocol documentation that should explains the correct message flow between the messages: connection_init, connection_ack and connection_keep_alive messages.

The one suggested by the current documentation is:

--> connection_init - 0s
<-- connection_ack - 0s
<-- ka . - 10s

Which is wrong.

The one suggested by this docs update, and the one that should be used is:

--> connection_init - 0s
<-- connection_ack - 0s
<-- ka . - 0s
<-- ka . - 10s

You can found more information about how I found this, in a discussion between myself and some graphcool guys here prisma/prisma1#309.

…ct workflow between connection init, connection ack and connection keep alive.
Copy link
Contributor

@NeoPhi NeoPhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. A little word smithing could be applied but in general this definitely clarifies.

@@ -21,7 +21,7 @@ export interface OperationMessage {
#### GQL_CONNECTION_INIT
Client sends this message after plain websocket connection to start the communication with the server

The server will response only with `GQL_CONNECTION_ACK` or `GQL_CONNECTION_ERROR` to this message.
The server will response only with `GQL_CONNECTION_ACK` + `GQL_CONNECTION_KEEP_ALIVE` (if used) or `GQL_CONNECTION_ERROR` to this message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While very unlikely it would be possible when keep alive is in use for the server to send GQL_CONNECTION_ACK and then GQL_CONNECTION_ERROR if for some reason the GQL_CONNECTION_KEEP_ALIVE failed to send.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if KEEP_ALIVE failed to send, ERROR will be failing as well...

Copy link
Contributor

@DxCx DxCx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@mistic mistic merged commit 4ca0f66 into master Oct 7, 2017
@mistic mistic deleted the fix-ka-message-on-protocol branch October 7, 2017 15:23
@mistic mistic mentioned this pull request Oct 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants