search

slack-go / slack

Slack API in Go, originally by @nlopes; Maintainers needed, contact @parsley42
https://pkg.go.dev/github.com/slack-go/slack
BSD 2-Clause "Simplified" License
4.63k stars 1.12k forks source link

SocketmodeHandler RunEventLoopContext method #1162

Closed lololozhkin closed 1 year ago

lololozhkin commented 1 year ago

Description

socketmodeHandler.RunEventLoop() function doesn't allow to provide context for cancellation, so I couldn't find any way to gracefully stop event handling from websocket.. One of the ways is to close handler.Client.Events channel, but it is dangerous due to possible panics in channel writes, it also not the only channel in this struct, and it is not guaranteed that websocket connection will be gracefully shut down :(

In other abstractions library has methods like RunContext() and I think it is good idea to write such a method for socketmodeHandler.

I could do it myself, but I aware of not propper shutting down the client (I coudn't find any close(client.Events) channel)

tylermmorton commented 1 year ago

It looks like that logic is somewhat in place but context.TODO() is used instead of exposing a context to be passed. I'm also experiencing this issue and would be willing to put up a PR for the fix.

https://github.com/slack-go/slack/blob/master/socketmode/socket_mode_managed_conn.go#L33

dblbirdie commented 1 year ago

Being able to pass a cancellable context is exactly what I am looking for. I would highly appreciate any effort in implementing this.

lololozhkin commented 1 year ago

Made pr with requested functionality, waiting for pull request to be approved.

kanata2 commented 1 year ago

Resolved by #1169