Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Honor deadman timer, fixes #1093 #1094

Merged
merged 3 commits into from Sep 11, 2022
Merged

Honor deadman timer, fixes #1093 #1094

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b69d0ea
Fail on context cancelled, retry on timeout
parsley42 Jul 28, 2022
d96792e
Update comment, remove reference to timeout
parsley42 Jul 28, 2022
9521295
Add back the waitgroup, just don't wait on ReadJSON
parsley42 Jul 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions socketmode/socket_mode_managed_conn.go
View file
Open in desktop
Expand Up @@ -123,9 +123,9 @@ func (smc *Client) run(ctx context.Context, connectionCount int) error {
}
}()

wg.Add(1)
// We don't wait on runMessageReceiver because it doesn't block on a select with the context,
// so we'd have to wait for the ReadJSON to time out, which can take a while.
go func() {
defer wg.Done()
defer cancel()

// The receiver reads WebSocket messages, and enqueues parsed Socket Mode requests to be handled by
Expand Down Expand Up @@ -162,7 +162,8 @@ func (smc *Client) run(ctx context.Context, connectionCount int) error {
return firstErr
}

// wg.Wait() finishes only after any of the above go routines finishes.
// wg.Wait() finishes only after any of the above go routines finishes and cancels the
// context, allowing the other threads to shut down gracefully.
// Also, we can expect firstErr to be not nil, as goroutines can finish only on error.
smc.Debugf("Reconnecting due to %v", firstErr)

Expand Down