From 01cba5b3e883184b23beabc391937f2375347390 Mon Sep 17 00:00:00 2001 From: Ashley Huynh Date: Fri, 23 Oct 2026 11:38:43 -0400 Subject: [PATCH 1/2] fix(socket_mode): seed last_ping_pong_time on first ping in builtin client The stale-connection check in `check_state()` was gated on `last_ping_pong_time is not None`, which is only set when a pong frame arrives. If the server never answered the first ping, the value stayed `None` forever and the client never disconnected. Fix by seeding `last_ping_pong_time` with the current time on the first `check_state()` ping, matching the existing behavior in the aiohttp client. Fixes #1997 Co-Authored-By: Claude Sonnet 4.6 Co-Authored-By: Claude --- slack_sdk/socket_mode/builtin/connection.py | 6 +++++- tests/slack_sdk/socket_mode/test_builtin.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/slack_sdk/socket_mode/builtin/connection.py b/slack_sdk/socket_mode/builtin/connection.py index 588f50497..f724ffaa0 100644 --- a/slack_sdk/socket_mode/builtin/connection.py +++ b/slack_sdk/socket_mode/builtin/connection.py @@ -249,7 +249,11 @@ def check_state(self) -> None: try: if self.sock is not None: try: - self.ping(f"{self.session_id}:{time.time()}") + ping_time = time.time() + self.ping(f"{self.session_id}:{ping_time}") + # Seed on first ping so the stale check fires even if no pong ever arrives. + if self.last_ping_pong_time is None: + self.last_ping_pong_time = ping_time except ssl.SSLZeroReturnError as e: self.logger.info( "Unable to send a ping message. Closing the connection..." diff --git a/tests/slack_sdk/socket_mode/test_builtin.py b/tests/slack_sdk/socket_mode/test_builtin.py index 6ba64c9a3..a4986a1d4 100644 --- a/tests/slack_sdk/socket_mode/test_builtin.py +++ b/tests/slack_sdk/socket_mode/test_builtin.py @@ -126,6 +126,25 @@ def run(): self.assertIsNotNone(conn) + def test_check_state_seeds_last_ping_pong_time_on_first_ping(self): + conn = Connection(url="wss://example.com", logger=self.logger) + conn.sock = patch("socket.socket").start() # non-None socket + self.assertIsNone(conn.last_ping_pong_time) + with patch.object(conn, "ping"): + conn.check_state() + self.assertIsNotNone(conn.last_ping_pong_time) + patch.stopall() + + def test_check_state_does_not_overwrite_existing_last_ping_pong_time(self): + conn = Connection(url="wss://example.com", logger=self.logger) + conn.sock = patch("socket.socket").start() + existing_time = time.time() - 1 + conn.last_ping_pong_time = existing_time + with patch.object(conn, "ping"): + conn.check_state() + self.assertEqual(conn.last_ping_pong_time, existing_time) + patch.stopall() + # ---------------------------------- # FrameHeader From 096c9caf10f67b64d245dd6328005d3ca55a9fc7 Mon Sep 17 00:00:00 2001 From: Ashley Huynh Date: Fri, 23 Oct 2026 11:54:06 -0400 Subject: [PATCH 2/2] test(socket_mode): use MagicMock directly instead of patch.start() in check_state tests Co-Authored-By: Claude Sonnet 4.6 Co-Authored-By: Claude --- tests/slack_sdk/socket_mode/test_builtin.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/slack_sdk/socket_mode/test_builtin.py b/tests/slack_sdk/socket_mode/test_builtin.py index a4986a1d4..7ade10f7c 100644 --- a/tests/slack_sdk/socket_mode/test_builtin.py +++ b/tests/slack_sdk/socket_mode/test_builtin.py @@ -3,7 +3,7 @@ import ssl import time import unittest -from unittest.mock import patch, sentinel +from unittest.mock import MagicMock, patch, sentinel from threading import Thread from slack_sdk import WebClient @@ -128,22 +128,20 @@ def run(): def test_check_state_seeds_last_ping_pong_time_on_first_ping(self): conn = Connection(url="wss://example.com", logger=self.logger) - conn.sock = patch("socket.socket").start() # non-None socket + conn.sock = MagicMock() self.assertIsNone(conn.last_ping_pong_time) with patch.object(conn, "ping"): conn.check_state() self.assertIsNotNone(conn.last_ping_pong_time) - patch.stopall() def test_check_state_does_not_overwrite_existing_last_ping_pong_time(self): conn = Connection(url="wss://example.com", logger=self.logger) - conn.sock = patch("socket.socket").start() + conn.sock = MagicMock() existing_time = time.time() - 1 conn.last_ping_pong_time = existing_time with patch.object(conn, "ping"): conn.check_state() self.assertEqual(conn.last_ping_pong_time, existing_time) - patch.stopall() # ---------------------------------- # FrameHeader