Skip to content

fix(sio-client): do not treat the ack as an error when retries is used without ackTimeout - #5584

Open
b84330808 wants to merge 1 commit into
socketio:mainfrom
b84330808:fix-retries-ack-without-timeout
Open

b84330808 wants to merge 1 commit into
socketio:mainfrom
b84330808:fix-retries-ack-without-timeout

Conversation

@b84330808

Copy link
Copy Markdown

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behavior

With retries but no ackTimeout, the client reads the server acknowledgement as an error:

// server
socket.on("echo", (arg, cb) => cb(arg));

// client
const socket = io({ retries: 3 });
socket.emit("echo", 42, (err, val) => {
  // err === 42, val === undefined, and the server handler ran 4 times
});
await socket.emitWithAck("echo", 42); // rejects with 42

_addToQueue() pushes a handler that expects the error as its first argument. Without a timeout, _registerAckCallback() stores it as-is. So onack() does not prepend null, and the packet is re-sent until the retries run out.

New behavior

The queue handler is flagged with withError, like the emitWithAck() handler and the wrapper used when a timeout is set. The callback now gets (null, 42) and the event is sent once. On disconnection, _clearAcks() now also notifies the queue handler, as it already does with ackTimeout. So a disconnection counts as one try, and the packet is still re-sent on reconnection.

Other information (e.g. related issues)

No existing issue found. I added two tests in test/retry.ts. Before the fix they fail with expected 1 to equal null, and emitWithAck() rejects with 2.

I used Claude Code to write this change.

…d without ackTimeout

With the `retries` option, the queue wraps the user callback in a handler
whose first argument is the error. Without `ackTimeout` (or `timeout()`),
that handler was registered as-is, so it received the server response
directly: `cb(42)` from the server was read as an error, the packet was
sent again until the retries were exhausted, and the user callback got
`42` as the error. `emitWithAck()` rejected with the response value.

The queue handler is now flagged with `withError`, so the response is
always prepended with `null`, like when `ackTimeout` is set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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