[INT-194] Only open slack socket on the persistent server #154
Conversation
lib/slack/slackApiConnection.ts
Outdated
| if (!app && isSlackEnabled) { | ||
| app = global.slack = new App({ | ||
| async function slackApiConnection() { | ||
| return new App({ |
There was a problem hiding this comment.
Are we sure there aren't places where this would get called twice per request?
(Also, why is this change necessary? Surely the persistent server has its own memory space so can have its own global slack client cache. If the only reason is to avoid calling app.start(), you could pass in a shouldStartApp = false parameter.)
There was a problem hiding this comment.
I didn't think that the global cache actually did much but am happy to revert it if that is the case. Having app.start() separate is preferable, as the boltjs example code shows listeners etc being declared before starting the app, which wouldn't be possible if the app was started before being returned.
There was a problem hiding this comment.
The main benefit of the global cache is not creating multiple Apps if slackApiConnection() is called more than once per request - I'm not sure if we do it (we shouldn't), but it's a decent precaution. It also saves on some overhead between requests (I'm not sure how expensive an App is to create).
|
Deployed a preview of this PR to https://pr-154-internal.dev.ystv.co.uk |
markspolakovs
deleted the
int-194-error-unhandled-event-server-disconnect-old-socket-in-state
branch
This should hopefully reduce the logged messages from the slack connection restarting.