Skip to content

fix(hulypulse-client): close socket when ping gets no pong - #11053

Open
RaphaelFakhri wants to merge 1 commit into
hcengineering:developfrom
RaphaelFakhri:fix/hulypulse-ping-timeout
Open

RaphaelFakhri wants to merge 1 commit into
hcengineering:developfrom
RaphaelFakhri:fix/hulypulse-ping-timeout

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Summary

Fixes the ping timeout in HulypulseClient so a socket that stops answering pings gets closed and reconnected.

Refs #10946 (first item only: the HulypulseClient ping timeout).

Problem

startPing() runs every 30 seconds. On each tick it cleared the previous ping timeout and created a new 5 minute one. Because the interval is shorter than the timeout, the timeout never fired. Even when it fired, the handler closed the socket only if readyState was not OPEN, so a dead but open socket was never closed and never reconnected.

Change

  • Keep the deadline of the oldest unanswered ping instead of resetting it on every tick.
  • Close the socket when the deadline passes without a pong. The existing onclose handler then runs the normal reconnect path.
  • Add client.test.ts with two tests that use fake timers and a fake WebSocket:
    • A socket that never answers is closed with code 1000 after the timeout.
    • A socket that answers every ping stays open.

Testing

The first test fails on the parent commit and passes with the change. The second test passes in both cases.

The presence client in plugins/presence-resources has the same pattern and is not changed here.

The ping timeout was cleared and recreated on every 30 second tick, so the 5 minute deadline never fired. The handler also only closed sockets that were no longer open. Keep the deadline of the oldest unanswered ping and close the socket when it fires.

Signed-off-by: Raphael Fakhri <153192858+RaphaelFakhri@users.noreply.github.com>

This branch has not been deployed

No deployments
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