⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@rbgmulmb
Copy link

Fix Acknowledgment Callback Signature with ackTimeout Option

Description

This PR fixes an inconsistency in acknowledgment callback signatures when emitting messages with the ackTimeout option set but without explicitly using .timeout().

Problem

When emitting a message with an acknowledgment callback before the 'connected' event with ackTimeout option configured, the callback was incorrectly receiving 2 arguments (null, response) instead of just the response. This behavior was inconsistent with messages emitted after connection, which correctly received only 1 argument.

This inconsistency made it difficult to write uniform code for handling acknowledgments.

Solution

The fix ensures that:

  1. Callback signatures remain consistent whether messages are emitted before or after connection
  2. The ackTimeout option only affects timeout behavior, not callback signature
  3. Callback signatures follow the standard Socket.IO patterns matching the acknowledgements documentation

Changes

  • lib/socket.ts: Refactored _registerAckCallback to introduce a hasTimeout attribute that differentiates between cases with explicit timeout (via .timeout()) and implicit timeout (via ackTimeout option)
  • test/socket.ts: Added comprehensive tests to verify correct callback signature behavior in various scenarios
  • test/node.ts: Fixed double done prevention using a flag guard to ensure done() callback is called exactly once in async tests with multiple completion paths
  • test/retry.ts: Added tests for timeout and disconnection scenarios

Behavior After Fix

const socket = io(BASE_URL, { ackTimeout: 5000 });

// Before connection - now consistent ✓
socket.emit("test", (response) => {
	console.log(response); // Single argument
});

socket.on("connect", () => {
	// After connection - already was correct ✓
	socket.emit("test", (response) => {
		console.log(response); // Single argument
	});
});

Standards Alignment

The new behavior matches the Socket.IO Acknowledgements documentation, ensuring consistent and predictable callback behavior across all scenarios.


Closes: #5446

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