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

fix(useWebSocket): don't hide pong setTimeout reference #2206

Merged
merged 8 commits into from Nov 9, 2022

Conversation

kellerza
Copy link
Contributor

@kellerza kellerza commented Sep 15, 2022

Description

In #2170 the capability was added to monitor pong's (or server replies) as part of the existing heartbeat/ping functionality.

Currently if the pong gets delayed/dropped for any reason (e.g. server restart) and "pong reply time">pingInterval; the ping function would execute again

The ping function creates a pong setTimout and saves the reference (this timeout will close the connection if allowed to expire).

If a previous setTimeout has not been cancelled yet, the reference is lost and the connection will be closed.

Additional context

By adjusting the timers appropriately you can now be more leanient and allow for "dropped" pongs. E.g if you require 1 pong (or server message) for every 3 pings you could set pongTimeout >= 3x ping interval

It's ok to clear and invalid setTimeout reference - https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout#notes


What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@kellerza kellerza changed the title Don't hide pong setTimeout reference fix(useWebSocket): don't hide pong setTimeout reference Sep 15, 2022
@antfu
Copy link
Member

antfu commented Sep 16, 2022

I think we'd better use undefined than 0

@kellerza
Copy link
Contributor Author

kellerza commented Oct 5, 2022

Anything still missing here?

@antfu antfu enabled auto-merge (squash) October 25, 2022 05:54
@antfu antfu merged commit 65f4341 into vueuse:main Nov 9, 2022
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

2 participants