yuya373 / emacs-slack

slack client for emacs
1.12k stars 117 forks source link

Conversations list can not be updated #555

Open isamert opened 3 years ago

isamert commented 3 years ago

Describe the bug I'm seeing bunch of [error] request--callback: peculiar error: 429 errors in messages buffer, every 3 seconds to be exact. I traced it and found out that the reason is the following:

[2021-09-02 10:41:47] [debug] Retrying Request After: 3 second, URL: https://slack.com/api/conversations.list, PARAMS: ((types . public_channel,private_channel,mpim,im) (cursor . dGVhbTpDMDFTNjBOSjZEUg==)) - ...

I tracked the logs and it seems like sometimes conversations.list returns no channels but a next cursor like the following:

[2021-09-02 12:32:39] [trace] REQUEST FINISHED. URL: "https://slack.com/api/conversations.list", PARAMS: (("types" . "public_channel,private_channel,mpim,im") ("cursor" . "dGVhbTpDMDFTQzVIUTQ2VA==")), DATA: (:ok t :channels nil :response_metadata (:next_cursor "dGVhbTpDMDFTQ0w0UFJRRg==")) - ...

Generally requesting the next cursor fails at this point and a retry loop begins. But this does not happen everytime, sometimes the next cursor request also returns an empty channel list and sometimes it returns some channels too. I'm not familar with slack's API, I don't know if this is something to be expected.

After a while, it gets into the retry loop and it becomes unable to pull all of my conversations, especially mpim ones. I terminated the retry loop by altering the behavior of slack-request on the fly and then changed slack-conversations-list function to only fetch for mpim conversations and it worked, at least for mpim conversations.

To Reproduce I believe the core of the issue is that I'm in a very big Slack organization with quite a lot of public channels. This causes a lot of cursor traversing and after a while Slack rate-limits my calls. Even though emacs-slack complies with the retry-after header that is returned by slack API, it still gets 429 response or there is something wrong with what the retry-after header supposed to mean. If it means you should wait this much seconds before doing any call to the API, emacs-slack does not correctly implement retry-after logic. If it means you should wait this much seconds before doing a request to this particular endpoint, then this is what emacs-slack implements. This page says that it's the latter but my experiments says it's the former.

Environment:

Additional context I'm sorry that I can not give you concrete steps to reproduce the issue but as I remarked earlier it's probably related to being in a very large Slack organization. Please let me know if there is anything I can provide you to debug this issue. I got a bit familiar with how emacs-slack works internally while trying to pin down the issue.

isamert commented 3 years ago

For the time being, I did the following change to the slack-conversations-list function to exclude public channels from the results:

@@ -284,10 +284,10 @@
         (channels nil)
         (groups nil)
         (ims nil)
-        (types (or types (list "public_channel"
-                               "private_channel"
-                               "mpim"
-                               "im"))))
+        (types (or types (list 
+                          "private_channel"
+                          "mpim"
+                          "im"))))
     (cl-labels
         ((on-success
           (&key data &allow-other-keys)

and it works (at the expense of not having public channels ofc).

dchenbecker commented 2 years ago

I think that for larger organizations we need some sort of local store that can act as a cache for channels. The current behavior of trying to fetch/load all in memory does not seem to scale well above a couple of thousand (I have > 50000 in my org)

ikapo commented 1 year ago

I too am experiencing this issue die to a large number of public channels in my org. Is there a way to only filter for specific public channels so that not all public channels are fetched? Maybe provide an option for a channel list?

Konubinix commented 1 year ago

Same here...

matthew-piziak commented 1 year ago

@Konubinix Yeah, I feel some something's changed recently. I'm getting a lot of Reconnecting... and request--callback: peculiar error: 429.

unhammer commented 1 year ago

Same here, just started happening a day or two ago. Not that many channels in my list (and I do get the full list of channels up, just for some reason seeing this error message in the logs a lot).

EDIT: (setq debug-on-message ".*request--callback: peculiar error: 429.*") gives :error: "ratelimited" (maybe there's some other bug that ends up looping requests? or slack api just started ratelimiting all clients?)

Debugger entered--Lisp error: "[error] request--callback: peculiar error: 429"
  message("%s" "[error] request--callback: peculiar error: 429")
  request--callback(#<killed buffer> :type "GET" :sync nil :params (("mpim_aware" . "1") ("presence_sub" . "true")) :data nil :files nil :headers (("Authorization" . "Bearer xoxc-...") ("Cookie" . "d=xoxd-...")) :parser slack-parse :success #f(compiled-function (&rest rest) #<bytecode -0x101b89121a5e3ca>) :error #f(compiled-function (&rest rest) #<bytecode -0x1fe0bf84505b2f48>) :timeout 30 :url "https://slack.com/api/rtm.connect?mpim_aware=1&pre..." :response #s(request-response :status-code 429 :history nil :data (:ok :json-false :error "ratelimited") :error-thrown (error http 429) :symbol-status nil :url "https://slack.com/api/rtm.connect?mpim_aware=1&pre..." :done-p nil :settings (:type "GET" :sync nil :params (("mpim_aware" . "1") ("presence_sub" . "true")) :data nil :files nil :headers (("Authorization" . "Bearer xoxc-...") ("Cookie" . "d=xoxd-...")) :parser slack-parse :success #f(compiled-function (&rest rest) #<bytecode -0x101b89121a5e3ca>) :error #f(compiled-function (&rest rest) #<bytecode -0x1fe0bf84505b2f48>) :timeout 30 :url "https://slack.com/api/rtm.connect?mpim_aware=1&pre..." :response #25 :encoding utf-8) :-buffer #<killed buffer> :-raw-header "HTTP/2 429 \ndate: Wed, 20 Sep 2023 10:38:27 GMT\nse..." :-timer nil :-backend curl) :encoding utf-8)
  apply(request--callback #<killed buffer> (:type "GET" :sync nil :params (("mpim_aware" . "1") ("presence_sub" . "true")) :data nil :files nil :headers (("Authorization" . "Bearer xoxc-...") ("Cookie" . "d=xoxd-...")) :parser slack-parse :success #f(compiled-function (&rest rest) #<bytecode -0x101b89121a5e3ca>) :error #f(compiled-function (&rest rest) #<bytecode -0x1fe0bf84505b2f48>) :timeout 30 :url "https://slack.com/api/rtm.connect?mpim_aware=1&pre..." :response #s(request-response :status-code 429 :history nil :data (:ok :json-false :error "ratelimited") :error-thrown (error http 429) :symbol-status nil :url "https://slack.com/api/rtm.connect?mpim_aware=1&pre..." :done-p nil :settings #3 :-buffer #<killed buffer> :-raw-header "HTTP/2 429 \ndate: Wed, 20 Sep 2023 10:38:27 GMT\nse..." :-timer nil :-backend curl) :encoding utf-8))
  request--curl-callback("https://slack.com/api/rtm.connect?mpim_aware=1&pre..." #<process request curl> "finished\n")
  apply(request--curl-callback ("https://slack.com/api/rtm.connect?mpim_aware=1&pre..." #<process request curl> "finished\n"))
  #f(compiled-function (&rest args2) #<bytecode -0xa59f46adfe56cb>)(#<process request curl> "finished\n")

I eventually get the channel list and can read messages, but when I send a message, it doesn't show up until I reload the channel (I've previously noticed this also will happen if my laptop has been sleeping or something and I open the buffer without first doing slack-start; the message gets sent, but I can't see that it's been sent; however as of the last few days that happens all the time.)

Konubinix commented 1 year ago

@unhammer I experience exactly the same behavior on my side. Also, I forgot to mention that we only have 16 channels in my company, so not that many.

So far, I have to fall back to using the slack web client to make sure I don't miss important messages.

Nazar65 commented 1 year ago

Hello there, It is now required to add cookie also to the websocket requests, it resolved the issue for me (url-cookie-store "d" "xoxd-xxxxx" nil ".slack.com" "/" t)

Since the emacs-slack used the websocket-open function and its' description says that it is using cookie from Cookies that are set via ‘url-cookie-store’ will be used during communication with the server, and cookies received from the server will be stored in the same cookie storage that the ‘url-cookie’ package uses. So adding d cookie to store resolve dthe issue for me with reconnecting as well

unhammer commented 1 year ago

What does that mean for multiple teams? (I tried storing the cookie of one team and it seems to have worked for all teams, but maybe I haven't tested well enough.)

Can/should slack-register-team just call url-cookie-store with the :cookie arg?

Nazar65 commented 1 year ago

@unhammer The (url-cookie-store) will work globaly for all requests that is using (websocket-open) function, so there is no way to separate them i believe, for the multiple teams probably code changes is required, to set correct cookie per team.

Nazar65 commented 1 year ago

@unhammer or try to set both cookie, maybe incorrect one will be rejected, but need to test

Konubinix commented 1 year ago

@Nazar65 Awesome. I could not try it yet, and I need to deal with the fact I have several communities, but now I know what I should focus on. Thank you a lot

ag91 commented 1 month ago

For the time being, I did the following change to the slack-conversations-list function to exclude public channels from the results:

That was a smart fix. The issue can be bypassed by setting a higher limit to slack-conversations-list like this. It seems better than selecting only a subset of channels because it allows search functionality to still work.