fix(socket-mode): observe reconnect failures and stop connecting after disconnect() by WilliamBergamin · Pull Request #2820 · slackapi/node-slack-sdk

WilliamBergamin · 2026-11-05T17:41:53Z

Summary

Fixes two Socket Mode connection bugs:

SocketModeClient reconnect rejection is unhandled when disconnect races retry #2656: disconnect() during a reconnect could crash the process. The reconnect rejected with undefined and nothing caught it. Now a reconnect stopped by disconnect() is ignored. A reconnect that fails for good still rejects with its cause.
socket-mode: Slack's disconnect (refresh_requested) leaves the app without a connection for 9–12 s #2767: apps went offline for 5 to 12s on every connection refresh. On a Slack disconnect message, the client now opens the new connection first and keeps the old one open until Slack closes it, so no events are dropped. (From fix(socket-mode): open the replacement connection before closing the old one on refresh #2821, merged into this branch.)

Related fixes:

disconnect() while start() is still connecting makes start() reject with the new SMConnectAbortedError, and no connection opens.
start() right after disconnect() opens one connection, not two.
test:coverage uses --enable-source-maps, so coverage points at TypeScript lines.

Supersedes #2657 and #2785; their authors are credited as co-authors.

⚠️ Trade-off: each refresh holds two connections for about 60s. Both count toward Slack's limit of 10 per app, so apps running many instances need room for one extra connection each. The changeset calls this out.

Testing

Unit tests for each race, each failing on main, plus integration tests with the real WebSocket client.
Manual, with a real Socket Mode app:
- stop() during a reconnect exits 0. A reconnect that hits invalid_auth exits 1 with the WebAPIPlatformError, same as main.
- Refresh gap is under 0.1s (was 5 to 12s). 6 refreshes, 288 events, 0 Slack retries, never more than 2 connections.
The repro script in SocketModeClient reconnect rejection is unhandled when disconnect races retry #2656 still reports an unhandled rejection because it fakes the race with Promise.reject(undefined). A real disconnect() doesn't.

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.

🤖 Generated with Claude Code

…r disconnect() delayReconnectAttempt chained the reconnect with .then(res) only, and the close handler dropped the returned promise, so a failed reconnect became an unhandled rejection that crashes Node by default. - Propagate the reconnect result (resolve and reject) and log failures from the close-triggered reconnect. - When disconnect() wins the race, reject with the new SMDisconnectedWhileConnectingError, which the close handler swallows, and check for it in start() right after apps.connections.open returns, so no WebSocket is opened after the user disconnected. Fixes #2656. Supersedes #2657. Co-authored-by: w3lld1 <42353747+w3lld1@users.noreply.github.com> Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>

changeset-bot · 2026-11-05T17:42:04Z

🦋 Changeset detected

Latest commit: e1ce02b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@slack/socket-mode	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2026-11-05T17:43:38Z

Codecov Report

❌ Patch coverage is 99.25926% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.28%. Comparing base (e9fcd0f) to head (e1ce02b).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2820      +/-   ##
==========================================
+ Coverage   93.93%   94.28%   +0.35%     
==========================================
  Files          66       67       +1     
  Lines       10581    10671      +90     
  Branches      611      680      +69     
==========================================
+ Hits         9939    10061     +122     
+ Misses        608      582      -26     
+ Partials       34       28       -6

Flag	Coverage Δ
cli-hooks	`94.16% <97.77%> (+0.31%)`	⬆️
cli-test	`94.16% <97.77%> (+0.31%)`	⬆️
logger	`94.16% <97.77%> (+0.31%)`	⬆️
oauth	`94.16% <97.77%> (+0.31%)`	⬆️
socket-mode	`94.16% <97.77%> (+0.31%)`	⬆️
web-api	`94.16% <97.77%> (+0.31%)`	⬆️
webhook	`94.16% <97.77%> (+0.31%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

….then delayReconnectAttempt is now async, so start() runs a microtask after the timer fires. The at-most-one-reconnect test awaits the clock, faking only setTimeout so tickAsync does not stall the node:test runner. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…t phase disconnect() during the WebSocket handshake rejected start() with undefined, so a stopped reconnect logged a spurious error. disconnect() during a WSS URL retry wait cleared the timer without settling the delay, so start() never settled. start() now rejects with SMDisconnectedWhileConnectingError whenever it is shutting down, and disconnect() ends the pending reconnect delay. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…MConnectAbortedError A shorter name that fits every path that throws it, the internal reconnect included, and matches the State.Connecting vocabulary. The error code becomes slack_socket_mode_connect_aborted_error. Unreleased, so no compatibility shim. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

A reconnect that fails for good (invalid_auth after a token revoke) was only logged, leaving the process up with no socket and nothing for the app to listen to. Emit State.Disconnected with the cause so apps and Bolt can react, as they could when the rejection crashed the process. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

When disconnect() landed while apps.connections.open was in flight and that call then failed recoverably, a fresh backoff was armed after disconnect() had already ended the pending one, so start() only rejected after clientPingTimeout * n ms. Bail before arming the wait. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

The handshake tests stubbed SlackWebSocket.connect, so disconnect() took the already-disconnected cleanup branch instead of closing a CONNECTING socket. Point them at a local TCP server that never upgrades. Closing a CONNECTING socket logs a WebSocket error (also on main), so the reconnect test now asserts only that the reconnect failure is not logged. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

A reconnect that fails for good used to crash with exit code 1, which restart-on-failure supervisors relied on. Now it logs, emits disconnected, and exits 0 when nothing else holds the event loop, or keeps running without a connection. Spell that out in the changeset with the opt-in to keep crashing. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

clearTimeout never settles the promise awaiting the timer, so disconnect() needed a stored resolver to end the wait. The abortable setTimeout from node:timers/promises does that directly and replaces both the timer handle and the resolver. A new test pins that disconnect() clears the pending delay right away, which keeps the close listener from ignoring the close event disconnect() itself triggers. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

… an alias Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

mwbrooks

✅ Thanks @WilliamBergamin! Small one: the PR description still names SMDisconnectedWhileConnectingError, but the code ships SMConnectAbortedError.

Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>

…es loud The close handler now ignores only SMConnectAbortedError, the disconnect() race from #2656. A reconnect that fails for good rejects with its cause, as on main, instead of being logged and emitted as `disconnected`. Bolt never listens for that event, so a default app exited 0 and on-failure supervisors stopped restarting it, and nested reconnects emitted `disconnected` twice. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

A SlackWebSocket nulls its inner socket before emitting close, so the current connection is never active when its own close arrives, and the identity check already ignores every other connection. The changeset no longer promises a `disconnected` error now that #2820 keeps a failed reconnect loud. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

The close handler now ignores a close from any connection other than the current one, by identity, instead of asking whether the current one is still active. Same outcome for the stale-close case from #2709, and it lets the next commits tell connections apart. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…ts URL A count of connects in flight covers the backoff wait and the apps.connections.open fetch, and drops once the new socket is assigned, so a failed handshake still reconnects through the close handler as before. A close during start()'s URL fetch no longer schedules a second connection. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

disconnect() aborts the session's AbortController and start() opens a new one, so a wait or URL fetch from before the restart sees its own session was stopped instead of reading a flag the restart already reset. This fixes disconnect(); start() during the reconnect wait or its URL fetch, which opened a second live connection. The per-wait reconnectDelay controller is gone. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…l start() The test no longer stubs start(): a reconnect whose URL fetch fails for good makes the close handler reject with that cause. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Against the mock servers, disconnect() and start() during the reconnect wait leave one connection open. The 409 test no longer swallows unhandled rejections: a reconnect stopped by disconnect() now settles quietly. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Each wait or URL fetch adds its own promise and removes exactly that one when it settles, so the busy check reads as "is any connect pending" and a holder can't release another's slot. Same behavior as the count. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

The wait uses try/catch instead of .catch(() => {}), and rethrows anything that isn't the abort. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Matches python-slack-sdk's connect_operation_lock, the sibling SDK's guard for the same job. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Without --enable-source-maps the coverage report gives line numbers of the compiled output, so codecov's patch check reads the wrong lines. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…tart A URL fetch still in flight from a session that disconnect() aborted kept connectOperations non-empty, so after start() the close handler skipped every reconnect of the new session. The stale fetch throws on its own aborted signal before it can connect, so the new session starts with an empty set. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

The two tests that held the WSS URL now use createClient's holdNextURL and a real connection drop instead of a second URL stub and a close event for a socket that never existed. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

SlackWebSocket's own tests already pin that disconnect() while CONNECTING emits close, so the client tests only need a socket that never says hello. The local TCP server that never upgraded, its socket bookkeeping, and the node:net import go away. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…name is gone - The waiting-reconnect test now asserts after the aborted wait settles, so it fails if the wait reconnects anyway. - The stale-close test is named for the identity check and ticks asynchronously, so it fails if a non-current close reconnects. - The multiple-close test closes a named current socket instead of an undefined one, and the fails-for-good test drops an unused readyState line. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

…old one on refresh (#2821) Co-authored-by: hippi345 <13539685+hippi345@users.noreply.github.com> Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>

zimeg

@WilliamBergamin Thanks for chasing these races down! Moving the reconnect wait onto an AbortSignal and acking each envelope on the connection it arrived on reads so much clearer 🔌 ✨ I left a question on a disconnect(); start() case and a thought on surfacing a reconnect that gives up, and neither is blocking.

📝 Before merge: #2821 landed on this branch, so I think the description and title might want to mention the #2767 refresh change too, since the squash commit carries both.

zimeg

@WilliamBergamin LGTM! Thanks for chasing these races down. Moving the reconnect wait onto an AbortSignal and acking each envelope on the connection it arrived on reads so much clearer 🔌 ✨ The notes are follow-ups for later only.

📝 Before merge: #2821 landed on this branch, so the description might mention the #2767 refresh change too, since the squash commit carries both.

zimeg · 2026-11-09T22:18:00Z

-      } else {
+      if (socket === this.retiringWebsocket) this.retiringWebsocket = undefined;
+      if (socket !== this.websocket) return;
+      if (this.shuttingDown || !this.autoReconnectEnabled) {


📝 note (non-blocking): A follow-up for later, since main behaves the same: an old socket's close is checked against the newest session, so disconnect(); start() without an await in between doesn't emit disconnected for the stop. The awaited path this PR fixes works well.

zimeg · 2026-11-09T22:18:00Z

+        await this.delayReconnectAttempt(() => this.start(), this.lifecycle.signal);
+      } catch (error) {
+        if (!(error instanceof SMConnectAbortedError)) {
+          throw error;


🧪 note (non-blocking): Rejecting with the cause when a reconnect fails for good matches main, so this is good to land. Later we might also emit disconnected with the error for apps that watch that event.

mwbrooks · 2026-11-09T23:05:02Z

+        return;
+      }
+      if (socket && socket !== this.websocket) return;
+      if (this.autoReconnectEnabled && !this.shuttingDown && event.reason !== 'link_disabled') {


question: Any disconnect reason other than link_disabled now opens a replacement right away with no backoff, so a reason we don't expect could loop fast on apps.connections.open. Would it be safer to skip the backoff only for warning and refresh_requested and send other reasons through the normal reconnect?

I incorporated this into the changes 💯 thanks for calling it out

Only `warning` and `refresh_requested` open a replacement connection right away. Any other reason, like `too_many_websockets`, closes the connection and reconnects after the usual delay, as on `main`, so an unexpected reason can't loop fast on `apps.connections.open`. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

WilliamBergamin mentioned this pull request Nov 5, 2026

fix(socket-mode): open the replacement connection before closing the old one on refresh #2821

Merged

2 tasks done

WilliamBergamin commented Nov 5, 2026

View reviewed changes

Comment thread packages/socket-mode/src/SocketModeClient.ts Resolved

refactor(socket-mode): use try/catch for the reconnect failure handler

362c891

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

WilliamBergamin added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch pkg:socket-mode applies to `@slack/socket-mode` labels Nov 5, 2026

WilliamBergamin and others added 5 commits November 5, 2026 13:25

Merge branch 'main' into fix-issue-2656

0c5efa5

Merge branch 'main' into fix-issue-2656

73a877e

WilliamBergamin marked this pull request as ready for review November 6, 2026 14:17

WilliamBergamin requested a review from a team as a code owner November 6, 2026 14:17

WilliamBergamin and others added 6 commits November 6, 2026 09:48

refactor(socket-mode): import setTimeout from timers/promises without…

501fea2

… an alias Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

mwbrooks approved these changes Nov 6, 2026

View reviewed changes

Comment thread packages/socket-mode/src/SocketModeClient.ts Resolved

Comment thread .changeset/socket-mode-reconnect-rejections.md Resolved

WilliamBergamin and others added 2 commits November 6, 2026 10:48

Update .changeset/socket-mode-reconnect-rejections.md

6815c93

Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>

WilliamBergamin added the semver:minor label Nov 6, 2026

WilliamBergamin added this to the socket-mode@next milestone Nov 6, 2026

WilliamBergamin removed the semver:patch label Nov 6, 2026

WilliamBergamin and others added 15 commits November 9, 2026 10:49

test(socket-mode): drive the fails-for-good reconnect through the rea…

931a3cc

…l start() The test no longer stubs start(): a reconnect whose URL fetch fails for good makes the close handler reject with that cause. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

docs(socket-mode): note the restart fix in the changeset

0b57999

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

refactor(socket-mode): catch only the aborted reconnect wait

5697065

The wait uses try/catch instead of .catch(() => {}), and rethrows anything that isn't the abort. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

refactor(socket-mode): name the pending connect set connectOperations

29d6250

Matches python-slack-sdk's connect_operation_lock, the sibling SDK's guard for the same job. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

test(socket-mode): map coverage to TypeScript source lines

41e1339

Without --enable-source-maps the coverage report gives line numbers of the compiled output, so codecov's patch check reads the wrong lines. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

fix(socket-mode): open the replacement connection before closing the …

dea9b45

…old one on refresh (#2821) Co-authored-by: hippi345 <13539685+hippi345@users.noreply.github.com> Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>

Merge branch 'main' into fix-issue-2656

4eadbb4

zimeg reviewed Nov 9, 2026

View reviewed changes

Comment thread packages/socket-mode/src/SocketModeClient.ts Resolved

Comment thread packages/socket-mode/src/SocketModeClient.ts Resolved

zimeg approved these changes Nov 9, 2026

View reviewed changes

mwbrooks reviewed Nov 9, 2026

View reviewed changes

WilliamBergamin merged commit 7bd6b38 into main Nov 10, 2026
22 checks passed

WilliamBergamin deleted the fix-issue-2656 branch November 10, 2026 16:10

This was referenced Nov 10, 2026

fix(socket-mode): reconnect immediately on Slack disconnect refresh #2785

Closed

fix(socket-mode): observe reconnect rejections #2657

Closed

socket-mode: Slack's disconnect (refresh_requested) leaves the app without a connection for 9–12 s #2767

Closed

Navigation Menu

Repository navigation

fix(socket-mode): observe reconnect failures and stop connecting after disconnect() - #2820

fix(socket-mode): observe reconnect failures and stop connecting after disconnect()#2820
WilliamBergamin merged 32 commits into
mainfrom
fix-issue-2656

WilliamBergamin commented Nov 5, 2026 •

edited

Loading

Uh oh!

changeset-bot Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

mwbrooks left a comment

Uh oh!

Uh oh!

Uh oh!

zimeg left a comment

Uh oh!

Uh oh!

Uh oh!

zimeg left a comment

Uh oh!

zimeg Nov 9, 2026

Uh oh!

zimeg Nov 9, 2026

Uh oh!

mwbrooks Nov 9, 2026

Uh oh!

WilliamBergamin Nov 10, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

WilliamBergamin commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Requirements

Uh oh!

changeset-bot Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

codecov Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

zimeg Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

mwbrooks Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

WilliamBergamin commented Nov 5, 2026 •

edited

Loading

changeset-bot Bot commented Nov 5, 2026 •

edited

Loading

codecov Bot commented Nov 5, 2026 •

edited

Loading