Skip to content

fix(eio,eio-client): do not close the connection while packets are still being received#5524

Open
GiHoon1123 wants to merge 1 commit into
socketio:mainfrom
GiHoon1123:fix-5450-ping-timeout-liveness-check
Open

fix(eio,eio-client): do not close the connection while packets are still being received#5524
GiHoon1123 wants to merge 1 commit into
socketio:mainfrom
GiHoon1123:fix-5450-ping-timeout-liveness-check

Conversation

@GiHoon1123

@GiHoon1123 GiHoon1123 commented Jul 13, 2026

Copy link
Copy Markdown

Summary

Fixes #5450.

The connection could be closed on a ping timeout even while other packets kept arriving, because only the pong (server side) or a fresh ping (client side) reset the timeout - a high-throughput connection could be dropped despite clear evidence it was still alive.

A previous fix for this (be7b4e7) was reverted (5359bae) for two reasons: it allocated a new timer on every incoming packet, and it delayed the next scheduled ping as a side effect (since only the timeout timer was touched, not the interval timer).

This takes a different approach:

  • A plain timestamp of the last received packet is tracked (updated on every packet, no timer touched on this path).
  • Only when the pre-existing ping-timeout timer actually fires does a check run: if a packet arrived recently enough, the timer is extended by the remaining time instead of closing; otherwise the connection is closed as before.
  • The ping/pong schedule itself (pingIntervalTimer on the server, the client's own ping-driven timer) is never touched, so there's no change to when pings are sent.

Both engine.io and engine.io-client needed the change. Delaying the server's wait for a pong also delays the server's next ping; without a matching client-side check, the client's own independent ping-timeout watchdog would still close the connection on its own once that delay exceeded pingTimeout. This was confirmed by testing the server-only fix in isolation - the client still disconnected on its own.

_hasPingExpired(), the synchronous check the Socket.IO client layer uses to detect a throttled/delayed timeout timer, is also updated to apply the same recent-packet check before deciding the connection is stale, since it's a separate path that can bypass the timer-based check entirely.

Test plan

  • New test: server does not close the connection when the client's pongs are delayed but other packets keep arriving (client's own watchdog neutralized to isolate server-side behavior)
  • New test: client does not close the connection when the server's pings are delayed but other packets keep arriving (server's own ping suppressed to isolate client-side behavior)
  • New test: _hasPingExpired() returns false when a packet was recently received, even if the throttled-timer deadline has passed
  • Existing engine.io suite passes for both protocol v4 and legacy v3 (EIO_CLIENT=3)
  • Existing engine.io-client suite passes
  • Verified regression coverage by reverting each fix individually (git stash on just that file) and confirming the corresponding new test fails without it, then restoring and reconfirming it passes

🤖 Generated with Claude Code

…ill being received

The server (and, symmetrically, the client) would close the connection on a
ping timeout even while other packets kept arriving, because only the pong
(or a fresh ping, on the client) reset the timeout - a high-throughput
connection could be dropped despite clear evidence it was still alive.

A previous fix for this (be7b4e7) was reverted (5359bae) for allocating a
new timer per packet and for delaying the next scheduled ping as a side
effect. This tracks a plain timestamp of the last received packet instead of
touching any timer on the common path, and only affects whether an
already-scheduled timeout check closes the connection or is extended - the
ping/pong schedule itself is untouched.

Both sides needed the fix: delaying the server's wait for a pong also delays
its next ping, so without a matching client-side check the client's own,
independent ping-timeout watchdog would still close the connection on its
own once the delay exceeded pingTimeout.

Also updates `_hasPingExpired()`, the synchronous check used by the
Socket.IO client layer to detect a throttled/delayed timeout timer, to apply
the same recent-packet check before deciding the connection is stale.

Fixes socketio#5450
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.

Heartbeat timeout is no longer reset on packet

1 participant