Skip to content

fix(adapter): do not let volatile packets break connection state recovery - #5539

Open
chuanghiduoc wants to merge 1 commit into
socketio:mainfrom
chuanghiduoc:fix/volatile-packets-break-recovery
Open

fix(adapter): do not let volatile packets break connection state recovery#5539
chuanghiduoc wants to merge 1 commit into
socketio:mainfrom
chuanghiduoc:fix/volatile-packets-break-recovery

Conversation

@chuanghiduoc

Copy link
Copy Markdown

Problem

When connectionStateRecovery is enabled, a volatile event whose last argument is a string breaks the recovery of the client that received it.

Reproduction (server + client from current main):

const io = new Server(httpServer, { connectionStateRecovery: {} });

io.on("connection", (socket) => {
  // ...
});

// after a normal broadcast has initialized the offset chain:
io.volatile.emit("status", "connected");

The client then disconnects and reconnects: socket.recovered is false, and every packet emitted during the disconnection is silently lost.

With the same scenario minus the io.volatile.emit(...) line, socket.recovered is true.

Cause

The two sides of the offset mechanism disagree about volatile packets:

  • server side (SessionAwareAdapter.broadcast()): a volatile EVENT packet gets no offset appended
  • client side (emitEvent()): any trailing string argument of a received event is stored as the offset

So after io.volatile.emit("status", "connected"), the client stores the literal string "connected" as its offset. On reconnection, restoreSession() does findIndex(packet.id === "connected"), which fails, and the session is dropped.

This also affects the cluster adapters, which share the same condition in addOffsetIfNecessary(). I have deliberately not touched them here, because their packet storage/replay lives in the external adapter packages and needs to be changed in sync.

Fix

In the in-memory adapter, volatile EVENT packets are now part of the offset chain like any other EVENT packet (they get an offset, and they are stored), but they are excluded from the replay in restoreSession(). Their semantics are unchanged - a volatile packet is still never delivered late.

Testing

  • new test in packages/socket.io-adapter/test/index.ts: session restore still succeeds with the volatile packet's offset, and the volatile packet is still excluded from missedPackets
  • npm test in packages/socket.io-adapter: 49 passing (48 before)
  • full suite in packages/socket.io: 216 passing

I also verified end-to-end with a script against the built packages: before the fix recovered: false / no replay; after the fix recovered: true / non-volatile packets replayed in order / volatile packets not replayed.

…very

The client stores the offset of every received event: it takes the last
item of the data array (see emitEvent() in the client). Volatile EVENT
packets did not get an offset appended, so if such a packet ended with a
string argument, the client stored that string as its offset. The next
restoreSession() call then failed to find it and the whole session was
dropped, silently discarding every packet emitted during the outage.

Volatile packets are now part of the offset chain like any other EVENT
packet, but they are still excluded from the replay in restoreSession(),
so their semantics are unchanged.
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