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

Support for GraphQL over WebSocket Ping and Pong message types #270

Closed
wants to merge 3 commits into from
Closed

Conversation

ExidCuter
Copy link

This PR implements the missing PING and PONG GraphQL WS messages for the client-side "heartbeat / keep alive" function.

Now the system does not close the connection when it receives a PING message, but sends a corresponding (PONG) message back, as specified in the graphql-ws protocol.

@pivotal-cla
Copy link

@ExidCuter Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@ExidCuter Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 25, 2022
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

I'm not sure WebGraphQlHandler should be involved. It's more of an internal API for invoking the WebInterceptor chain, it's not meant to be implemented by an application. I think for now we would simply respond to the the ping with a pong. The payload on the pong is optional.

@ExidCuter
Copy link
Author

Hi, I'm not quite sure I understand your comment. Do you mean I should remove handleWebSocketPing and handleWebSocketPong from the WebGraphQlHandler interface and just respond directly with Mono?

I added methods to WebGraphQlHandler because the PING and PONG messages are GraphQL-WS message types and I thought they should be implemented the same way as the others.

@rstoyanchev
Copy link
Contributor

Yes, I meant that those methods should be removed from WebGraphQlHandler. They don't serve any purpose and they don't help to handle PING messages. For now we would automatically handle those.

@rstoyanchev rstoyanchev self-assigned this Feb 8, 2022
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 8, 2022
@rstoyanchev rstoyanchev added this to the 1.0.0-M6 milestone Feb 8, 2022
@rstoyanchev rstoyanchev changed the title Implemented missing PING and PONG messages described in https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md#ping Support for GraphQL over WebSocket Ping and Pong message types Feb 8, 2022
@rstoyanchev
Copy link
Contributor

The code has evolved since, so I've applied the equivalent change manually, also on the client side since these are bidrectional messages. Thanks for the PR in any case!

@andrey-nakin
Copy link

@rstoyanchev

  • Does server automatically reply to PING messages sent from the client?
  • Can server send PING messages to the client periodically? Is there any configuration property for this?

@rstoyanchev
Copy link
Contributor

Yes the server replies automatically. There isn't a way to send pings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Ping and Pong messages from the GraphQL over WebSocket protocol
5 participants