[hello-ashleyintech]

Summary

Fixes a bug where the builtin Socket Mode client never disconnected if the server never sent a first pong. Closes #1997.

The stale-connection check in check_state() was gated on last_ping_pong_time is not None, but that field is only ever set when a pong frame arrives. If the server never answered the first ping, the value stayed None forever and the client kept sending pings indefinitely without reconnecting.

Fix: seed last_ping_pong_time with the current time on the first check_state() ping if it hasn't been set yet, so the ping_interval * 4 stale check can fire even without a first pong. This matches the existing behavior in the aiohttp-based client.

Testing

• test_check_state_seeds_last_ping_pong_time_on_first_ping — asserts last_ping_pong_time is set after the first check_state() call when no pong has arrived
• test_check_state_does_not_overwrite_existing_last_ping_pong_time — asserts an existing value from a real pong is not reset by subsequent check_state() calls
• Run ./scripts/run_tests.sh tests/slack_sdk/socket_mode/test_builtin.py to verify all tests pass

Category

• slack_sdk.web.WebClient (sync/async) (Web API client)
• slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
• slack_sdk.socket_mode (Socket Mode client)
• slack_sdk.signature (Request Signature Verifier)
• slack_sdk.oauth (OAuth Flow Utilities)
• slack_sdk.models (UI component builders)
• slack_sdk.scim (SCIM API client)
• slack_sdk.audit_logs (Audit Logs API client)
• slack_sdk.rtm_v2 (RTM client)
• /docs (Documents)
• /tutorial (PythOnBoardingBot tutorial)
• tests/integration_tests (Automated tests for this library)

Requirements

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.
• I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.35%. Comparing base (6a21a83) to head (096c9ca).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1998   +/-   ##
=======================================
  Coverage   84.35%   84.35%           
=======================================
  Files         118      118           
  Lines       13793    13796    +3     
=======================================
+ Hits        11635    11638    +3     
  Misses       2158     2158           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[WilliamBergamin]

Thanks for tracking this down! 🙇 🙏

The fix works: with the #1997 repro, the client now reconnects about 3s in at ping_interval=0.5, where 3.45.0 never does. CI is green too.

A few things before merging, details inline:

• Keep last_ping_pong_time pong-only. It's technically public, and apps read it as "a pong arrived recently". A private baseline for the stale check fixes the bug without changing what it reports.
• Test the actual failure mode: the stale disconnect firing when no pong ever arrives.
• disconnect() race: this change makes an existing race in disconnect() easy to hit on close().

[WilliamBergamin]

What do you think about adding a test that covers the bug in the issue: with no pong ever arriving, the stale check disconnects?
For example, patch slack_sdk.socket_mode.builtin.connection.time.time with an advancing side_effect, call check_state() until we're past ping_interval * 4, and assert conn.sock is None. It might also be worth asserting that the seeded value equals the timestamp sent in the ping payload, not just that it's not None.

[WilliamBergamin]

disconnect() checks self.sock is not None outside the locks, so the monitor thread's stale-disconnect and a user's close() can both pass the check, and the second one hits None.close(). Before this PR the no-pong case never reached the stale path. Now the #1997 repro followed by client.close() raises AttributeError: 'NoneType' object has no attribute 'close' every time for me. Moving the check inside both locks fixed it locally:

def disconnect(self) -> None:
    with self.sock_send_lock:
        with self.sock_receive_lock:
            if self.sock is not None:
                self.sock.close()
                self.sock = None
    self.logger.info(f"The connection has been closed (session id: {self.session_id})")

Happy for this to be a follow-up PR if you'd rather keep this one focused! 🤔

[WilliamBergamin]

last_ping_pong_time is public, and today only a pong ever sets it, so apps use it as a liveness signal. For example, a health check like pong is not None and time.time() - pong < N would report a never-ponged connection as healthy for ~20s with this change. Could we leave it pong-only and give the stale check its own baseline?

# __init__
self._first_ping_time: Optional[float] = None

# check_state
ping_time = time.time()
self.ping(f"{self.session_id}:{ping_time}")
if self._first_ping_time is None:
    self._first_ping_time = ping_time
...
stale_check_since = self.last_ping_pong_time if self.last_ping_pong_time is not None else self._first_ping_time
if stale_check_since is not None:
    disconnected_seconds = int(time.time() - stale_check_since)

Setting it after ping() means the clock only starts once a ping actually goes out, and it needs to be set once, not on every ping. Otherwise its age never exceeds one interval and the stale check never fires.