Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

FIX RTM RETRY AND RECONNECT LOGIC #532

Closed
wants to merge 1 commit into from

Conversation

inn0v8
Copy link

@inn0v8 inn0v8 commented Dec 5, 2016

Existing Botkit slackbot reconnect logic is failing. The retry attributes are passed along with the botkit object but slackbot_worker.js is looking for them in the bot object. Because these attributes do not exist the retry will always fail. There have been several issues opened against this problem and custom code has been written to try to manage the scenario with there is an RTM disconnect and the bot essentially dies for that team because it cannot reconnect.

This fix points to the botkit object in slack_worker.js which then allows the retry logic to work as planned. Since making this change the bot will automatically reconnect and use the reconnect attributes that are used when the controller is initiated.

Existing Botkit slackbot reconnect logic is failing. The retry attributes are passed along with the botkit object but slackbot_worker.js is looking for them in the bot object. Because these attributes do not exist the retry will always fail. There have been several issues opened against this problem and custom code has been written to try to manage the scenario with there is an RTM disconnect and the bot essentially dies for that team because it cannot reconnect.

This fix points to the botkit object in slack_worker.js which then allows the retry logic to work as planned. Since making this change the bot will automatically reconnect and use the reconnect attributes that are used when the controller is initiated.
@jonchurch
Copy link
Contributor

jonchurch commented Dec 11, 2016

@benbrown This looks like the same change I have made in #514, looking for config vars on botkit.config instead of bot.config

The docs specify passing retrywhen spawning a bot, and changing the logic in this way could be breaking for some folks.

But I think most people have been trying to pass their retry options into the controller or configureSlackApp({}) because the spawning logic for Slack apps out of the box is abstracted away into getting the config from the DB then connecting all existing bots.

I'd propose changing convention so bot.spawn() only deals with identifying a bot, giving it either a token or a team config with bot.identity. Options like retry and send_via_rtm seem like global options for the controller to me, and that's where I naturally want to pass them into.

@gunar
Copy link

gunar commented Dec 15, 2016

Looking forward to this.

@benbrown
Copy link
Contributor

I agree @jonchurch those type of options belong at the controller level.

However, for backwards compatibility and also to handle some multi-team bots, we should allow it to be passed in at the bot level too.

I'll look int the code and see what needs to happen to make this mergable.

@benbrown
Copy link
Contributor

OK! I did this in a slightly different way, but should be good to go.

@benbrown benbrown closed this Dec 19, 2016
@gunar
Copy link

gunar commented Dec 20, 2016

@benbrown do you mean here?

@benbrown
Copy link
Contributor

@gunar yes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants