Skip to content

Commit

Permalink
fix: send graphql-specific ping instead of ws ping frame (#117)
Browse files Browse the repository at this point in the history
## Description

When using the new keepalive functionality, we noticed that the pings
are sent as [websocket ping frames][1] rather than as
[graphql-transport-ws ping messages][2].

Aside from conforming to the protocol, the sub-protocol ping messages
should be used to support a heartbeat mechanism from browser (i.e. wasm)
clients, where there isn't a concept of ping / pong frames.
Specifically, setting a `KeepAliveSettings::interval` from a browser
client does not do anything right now.

## Testing

Locally tested to confirm that the correct ping messages are now sent
from browser clients (and that a gql-conforming server responds with the
appropriate pong message).

[1]: https://datatracker.ietf.org/doc/html/rfc6455#section-5.5.2
[2]: https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md#ping
  • Loading branch information
szgupta authored and obmarg committed Aug 21, 2024
1 parent a4e7eb8 commit bb958cb
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@ all APIs might be changed.

- Fixed ping responses not following the graphql-transport-ws protocol
([#116](https://github.com/obmarg/graphql-ws-client/pull/116))
- `graphql-transport-ws` ping messages are now sent, instead of websocket ping
frames, when using the `KeepAliveSettings` ([#117](https://github.com/obmarg/graphql-ws-client/pull/117))

### Contributors

Thanks to the people who contributed to this release:

- @vorporeal
- @szgupta

## v0.10.1 - 2024-06-08

Expand Down
2 changes: 1 addition & 1 deletion src/next/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl ConnectionActor {
code: Some(code),
reason: Some(reason),
}),
ConnectionCommand::Ping => Some(Message::Ping),
ConnectionCommand::Ping => Some(Message::graphql_ping()),
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/next/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ impl Message {
Self::Text(serde_json::to_string(&crate::protocol::Message::Pong::<()>).unwrap())
}

pub(crate) fn graphql_ping() -> Self {
Self::Text(serde_json::to_string(&crate::protocol::Message::Ping::<()>).unwrap())
}

pub(crate) fn complete(id: usize) -> Self {
Self::Text(
serde_json::to_string(&crate::protocol::Message::Complete::<()> { id: id.to_string() })
Expand Down
2 changes: 2 additions & 0 deletions src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub enum Message<'a, Operation> {
Subscribe { id: String, payload: &'a Operation },
#[serde(rename = "complete")]
Complete { id: String },
#[serde(rename = "ping")]
Ping,
#[serde(rename = "pong")]
Pong,
}
Expand Down

0 comments on commit bb958cb

Please sign in to comment.