socketio / socketio/socket.io

Incorrect acknowledgment callback signature when emitting before connection with ackTimeout option

Open
#5,446 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

question
Dominant language
TypeScript
Stars
63.2k
Forks
10.3k
Avg merge
11d 20h
Merged PRs (30d)
2

Description

Description

When emitting a message with an acknowledgment callback before the 'connected' event, and the socket is created with the ackTimeout option (but without explicitly using .timeout()), the callback receives an incorrect number of arguments.

Expected Behavior

When emitting a message with an acknowledgment callback without explicitly using .timeout(), the callback should receive only the server response as a single argument, regardless of whether the message is sent before or after the 'connected' event.

const socket = io(BASE_URL, {
  ackTimeout: 5000  // This should only affect timeout behavior, not callback signature
});

// Emitting before connection
socket.emit("test", (response) => {
  // Should receive: response (1 argument)
  console.log(response);
});

socket.on("connect", () => {
  // Emitting after connection
  socket.emit("test", (response) => {
    // Should receive: response (1 argument)
    console.log(response);
  });
});

Actual Behavior

When emitting before the 'connected' event with ackTimeout option set, the callback receives 2 arguments: (null, response) instead of just (response). This is inconsistent with emitting after the 'connected' event, where the callback correctly receives only 1 argument: (response).

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

// Emitting before connection - INCORRECT BEHAVIOR
socket.emit("test", (err, response) => {
  // Receives: (null, response) - 2 arguments
  // Expected: (response) - 1 argument
  console.log(err);      // null
  console.log(response); // actual response
});

socket.on("connect", () => {
  // Emitting after connection - CORRECT BEHAVIOR
  socket.emit("test", (response) => {
    // Receives: (response) - 1 argument ✓
    console.log(response);
  });
});

Root Cause

The issue occurs in the _registerAckCallback method in lib/socket.ts. When ackTimeout is set (but this.flags.timeout is not explicitly set via .timeout()), the code wraps the callback and sets withError: true. This causes the onack method to prepend null to the response data, resulting in the callback being called with (null, ...responseArgs) instead of (...responseArgs).

The withError flag should only be set when the user explicitly uses .timeout(), not when only the ackTimeout option is configured. The ackTimeout option should only affect timeout behavior (i.e., when to trigger a timeout error), not the callback signature.

Steps to Reproduce

  1. Create a socket with ackTimeout option (but don't use .timeout())
  2. Emit a message with an acknowledgment callback before the 'connected' event
  3. Observe that the callback receives 2 arguments: (null, response)
  4. Emit the same message after the 'connected' event
  5. Observe that the callback receives 1 argument: (response)

Impact

This inconsistency makes it difficult to write code that handles acknowledgments uniformly, as developers must account for different callback signatures depending on when the message is emitted relative to the connection event.

Proposed Fix

Modify _registerAckCallback to only set withError: true when this.flags.timeout is explicitly set (via .timeout()), not when only ackTimeout option is configured. The timeout timer should still be set up for ackTimeout, but the callback signature should remain unchanged.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/socket.ts at _registerAckCallback and follow the onack path that handles acknowledgment arguments. Reproduce emissions before and after the connected event with ackTimeout configured, then verify both callbacks receive the same single response argument while explicit .timeout() behavior remains distinct.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
api
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.