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

chore: simplify keep alive implementation #94

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

obmarg
Copy link
Owner

@obmarg obmarg commented May 22, 2024

A small tweak to #93 to avoid duplicating logic

@obmarg obmarg force-pushed the obmarg/push-vpnkuwqwxmno branch from f539c0c to 986cc73 Compare May 22, 2024 22:29
@obmarg obmarg enabled auto-merge (squash) May 22, 2024 22:30
@obmarg obmarg merged commit c621e5e into main May 22, 2024
2 checks passed
@obmarg obmarg deleted the obmarg/push-vpnkuwqwxmno branch May 22, 2024 22:30
obmarg added a commit that referenced this pull request Jun 8, 2024
#93 & #94 added a notion of keep alives to graphql-ws-client. But the
implementation caused the connection to drop whenever no messages were
received for the specified period. This isn't how I'd usually expect a
keep alive to work though - I'd usually expect it to send pings during
inactive periods, and only drop the connection if a few of those pings
were not replied to.

This PR implements that behaviour instead. It's a slightly breaking
change over the earlier implementation, but that was not yet released so
that seems fine.
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.

1 participant