Skip to content

fix(socket-mode): terminate closing connections earlier if normal close responses fail#2565

Merged
zimeg merged 4 commits intomainfrom
zimeg-fix-socket-mode-close-terminate
Apr 29, 2026
Merged

fix(socket-mode): terminate closing connections earlier if normal close responses fail#2565
zimeg merged 4 commits intomainfrom
zimeg-fix-socket-mode-close-terminate

Conversation

@zimeg
Copy link
Copy Markdown
Member

@zimeg zimeg commented Apr 29, 2026

Summary

This PR terminates closing connections earlier if normal close responses fail for openclaw/openclaw#72808 🦞

We hope to attempt a reconnection sooner because these reconnection issues can occur for various reasons, including during a server swap on the backend:

📚 https://docs.slack.dev/apis/events-api/using-socket-mode#disconnect

Preview

Before

before.mov

After

after.mov

Reviewers

Please build this branch and test with a Bolt app ⚡

[INFO]  bolt-app ⚡️ Bolt app is running!

After connecting with Socket Mode, turn internet off, grab a glass of water, and turn the internet back on. A reconnection should succeed!

Notes

  • Sometimes num_connections returns as "2" instead of "1" after reconnections. I believe this can happen both before and after these changes, but soon resolves after Slack decides the connection is idle 🔬
  • We might prefer the sooner termination because errors in initial retries are not common to resolve if network issues exist. We prefer to attempt reconnecting sooner because of this!

Requirements

@zimeg zimeg added this to the socket-mode@next milestone Apr 29, 2026
@zimeg zimeg self-assigned this Apr 29, 2026
@zimeg zimeg requested a review from a team as a code owner April 29, 2026 18:53
@zimeg zimeg added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch area:performance issues where performance is a meaningful concern pkg:socket-mode applies to `@slack/socket-mode` labels Apr 29, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 29, 2026

🦋 Changeset detected

Latest commit: f1b9280

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 Patch

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
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.38%. Comparing base (62c5a3f) to head (f1b9280).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2565      +/-   ##
==========================================
- Coverage   87.41%   87.38%   -0.04%     
==========================================
  Files          62       62              
  Lines       10251    10256       +5     
  Branches      415      415              
==========================================
+ Hits         8961     8962       +1     
- Misses       1269     1273       +4     
  Partials       21       21              
Flag Coverage Δ
cli-hooks 87.38% <100.00%> (-0.04%) ⬇️
cli-test 87.38% <100.00%> (-0.04%) ⬇️
logger 87.38% <100.00%> (-0.04%) ⬇️
oauth 87.38% <100.00%> (-0.04%) ⬇️
socket-mode 87.38% <100.00%> (-0.04%) ⬇️
web-api 87.38% <100.00%> (-0.04%) ⬇️
webhook 87.38% <100.00%> (-0.04%) ⬇️

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.

Copy link
Copy Markdown
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 💯

Left once comment on the tests but it should be none blocking

Comment thread packages/socket-mode/test/integration.test.js
} else if (this.websocket.readyState === WebSocket.CLOSING) {
// A close frame was already sent but the peer hasn't responded. Force-terminate rather than
// waiting for the ws library's closeTimeout (~30s) while the ping monitor logs repeated warnings.
this.logger.debug('Terminating WebSocket (close frame sent but no response, force-terminating).');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 💯

@zimeg
Copy link
Copy Markdown
Member Author

zimeg commented Apr 29, 2026

@WilliamBergamin I appreciate the review and suggestions toward this change! Let's merge this now to release upcoming 🎁

@zimeg zimeg enabled auto-merge (squash) April 29, 2026 23:26
@zimeg zimeg merged commit 5395b0c into main Apr 29, 2026
18 of 20 checks passed
@zimeg zimeg deleted the zimeg-fix-socket-mode-close-terminate branch April 29, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:performance issues where performance is a meaningful concern bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented pkg:socket-mode applies to `@slack/socket-mode` semver:patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants