Skip to content

Support graphql-transport-ws websocket protocol #539

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

Merged
merged 12 commits into from
Apr 13, 2023
Merged

Conversation

rose-a
Copy link
Collaborator

@rose-a rose-a commented Apr 11, 2023

This PR picks up #493, rebases on current master, adds unit tests, resolves the errors and does some refactoring.

Not yet implemented:

  • protocol auto-negotiation (needs a complete refactoring of this PRs changes which separates websocket and protocol handling to be able to select the protocol from within the GraphQLHttpWebSocket class)
  • client-side ping

@Shane32 Regarding ping/pong whats your opinion on how the payload of a ping should be handled? Its not explicitly stated in the spec, but my current implementation reflects the payload of the ping request in the corresponding pong request.
Since the spec explicitly allows sending pongs at any time, this allows the server to correlate pongs with sent pings to use this for metrics etc.

Sorry, something went wrong.

joao-avelino and others added 9 commits April 11, 2023 10:59

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…pWebSocket into two classes (one per protocol) that inherit from BaseGraphQLHttpWebSocket
@rose-a rose-a mentioned this pull request Apr 11, 2023
@Shane32
Copy link
Member

Shane32 commented Apr 11, 2023

how the payload of a ping should be handled

reflects the payload of the ping request

I think that's a good idea. I could implement the same in GraphQL.NET Server 7, although it is not so now.

@Shane32
Copy link
Member

Shane32 commented Apr 11, 2023

Just FYI, when server-side keep-alive is enabled within GraphQL.NET Server 7, only pongs are sent, and only when the specified amount of time has elapsed from the last sent or received message.

@rose-a rose-a requested a review from sungam3r April 12, 2023 11:33
@rose-a
Copy link
Collaborator Author

rose-a commented Apr 12, 2023

@sungam3r pls review for code sanity ;)
@Shane32 thanks for your input

@gjedlicska
Copy link

gjedlicska commented May 23, 2023

Any plans to support protocol auto negotiations @rose-a ?

@rose-a
Copy link
Collaborator Author

rose-a commented May 23, 2023

Already implemented and enabled by default in v6.0.0

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

Successfully merging this pull request may close these issues.

None yet

3 participants