-
Notifications
You must be signed in to change notification settings - Fork 663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@slack/socket-mode - Handle event.type === disconnect && reason === 'too_many_websockets' #1654
Comments
Hello! Thanks for reporting. What would the expectation be in this scenario? What should |
Forgot the log... 😅
|
The expected behavior would be for the app to catch the error when it receives:
|
OK, so catch the error and I guess not attempt to re-connect? |
@filmaj I think the reconnect behavior is still necessary given the app would enter into a disconnected state forever. It's worth noting that in my app logs, I only see |
I believe we only allow 10 concurrent websocket connections per app, so if Slack is returning "too many websockets", it makes sense to me to reduce the amount of reconnect attempts - otherwise the warning/error will continue to be raised, and we would essentially be stampeding on the That's what I'm trying to tease out asking these questions and having this discussion: what should the proper behaviour be? 🤔 |
Ah I see - I wasn't thinking too clearly on the implications of max limit imposed by the platform. I agree with your suggestion, the app should catch the error, then enter disconnected state. At this point the app owner can decide further logic (reboot the app via healthcheck, etc) |
👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized. |
As this issue has been inactive for more than one month, we will be closing it. Thank you to all the participants! If you would like to raise a related issue, please create a new issue which includes your specific details and references this issue number. |
any updates? have the same problem |
Apologies for no updates here and that I disappeared... let me look into this again after all this time. 🙇 |
Thank you) |
@dbudim hmm, by changing your token you avoid the issue? That doesn't make sense to me.. which token are you replacing? Your bot token or your application token? Do you have more than one instance of the app running? |
Single instance of app, replacing application token and problem is gone for a week😅 |
@dbudim do the logs look similar to what the original poster posted? As in, first you see a |
Yes, same situation java SDK:
|
Oh, you are using the Java SDK, good to know. |
I've posted your issue to the java-slack-sdk repo: slackapi/java-slack-sdk#1223 Best to follow that issue along since the code for the node socket-mode library is different from the java bits. |
@seratch since I am looking at doing a major new release of the socket mode package, do you have any thoughts on this issue? If Slack raises "too many websockets" - what should this program do? I feel like continuing to attempt to reconnect would be creating unnecessary, likely-to-fail connection attempts. Do you have opinions on what the package should do? |
@filmaj thank you for your detailed investigation of the scenario. I agree handling the 'too_many_websockets' reason pattern in a proper way could be a nice improvement. In reality, the only action SocketModeClient can take is to patiently wait until the Slack server permits a new connection. That being said, modifying the behavior in this manner should certainly be better than currently treating the pattern as a "slack_event" for sure. |
An RC was released to try to address this: |
The fix has now been released to mainline in 1.3.5 and is out of release candidacy. It will also be present in the soon-to-come 2.0 release of |
@filmaj I seem to be hitting the following now on 1.3.5:
With debug logging it shows:
|
@MadrMan I filed #1787 to track this. Worth noting that, while this is a legit problem and we will address it in this package, you should also consider why Slack is telling you you have too many parallel websocket connections open with Slack (10 is the maximum allowed). If you can get the number of parallel websocket connections for this app down under 10, you would mitigate seeing this issue. |
👋 from Slack Customer Success.
Current state of @slack/socket-mode package does not account for
event.type === disconnect && reason === 'too_many_websockets'
, causing unhandled error to bubble up when Slack decides to disconnect existing connection duringconnecting:handshaking
stateHere's a log output - the reconnect happens initially after the app receives
event.type === 'disconnect' && reason === 'warning'
Packages:
Select all that apply:
@slack/web-api
@slack/rtm-api
@slack/webhooks
@slack/oauth
@slack/socket-mode
@slack/types
Requirements
Feature request: have the app catch
reason === 'too_many_websockets'
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
The text was updated successfully, but these errors were encountered: