Skip to content

Subscription refactor #512

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

Closed
wants to merge 2 commits into from
Closed

Conversation

smyrick
Copy link
Contributor

@smyrick smyrick commented Dec 11, 2019

📝 Description

Refactor of the websocket logic to keep sending the keep alive messages until the client calls connection terminate.

Clients can open a connection with GQL_CONNECTION_INIT but never call GQL_START. The keep alive messages from the server should still be sent in that case, which also means we don't need to save the websocket session. We can just stop the session only on GQL_CONNECTION_TERMINATE

This also cleans up the keep alive connections after GQL_STOP is sent by the client or the server sends GQL_CONNECTION_ERROR

https://github.com/apollographql/subscriptions-transport-ws/blob/master/PROTOCOL.md

🔗 Related Issues

@smyrick smyrick added changes: patch Changes require a patch version type: refactor Code changes that have no impact on users labels Dec 11, 2019
@smyrick smyrick changed the title Subscription perf Subscription refactor Dec 11, 2019
@codecov-io
Copy link

Codecov Report

Merging #512 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #512      +/-   ##
============================================
+ Coverage      97.9%   97.98%   +0.08%     
  Complexity      335      335              
============================================
  Files           106      106              
  Lines          1290     1292       +2     
  Branches        207      206       -1     
============================================
+ Hits           1263     1266       +3     
  Misses            7        7              
+ Partials         20       19       -1
Impacted Files Coverage Δ Complexity Δ
...ing/execution/ApolloSubscriptionProtocolHandler.kt 96.61% <100%> (+1.87%) 12 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 111ccaf...4786d32. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes: patch Changes require a patch version type: refactor Code changes that have no impact on users
Development

Successfully merging this pull request may close these issues.

2 participants