hisabimbola / slack-history-export

A NPM module that allows slack users export their history
MIT License
287 stars 45 forks source link

Rate-limiting not working on batch download #39

Closed devoid closed 6 years ago

devoid commented 7 years ago

First, thank you for making this tool! It works really well even for downloading large individual DM histories (e.g. multi-megabyte JSON outputs). I've written a small bash script to periodically download archives of DMs from multiple teammates:

#!/bin/bash
dms=( alice bob charlie )
for user in ${dms[@]}; do
    slack-history-export -t $SLACK_TOKEN -T dm -u $user -f $user.json
done

But after the first few work flawlessly, I start getting rate-limiting errors:

Error: Too Many Requests
    at Request.callback (/usr/local/lib/node_modules/slack-history-export/node_modules/superagent/lib/node/index.js:698:17)
    at IncomingMessage.<anonymous> (/usr/local/lib/node_modules/slack-history-export/node_modules/superagent/lib/node/index.js:922:12)
    at emitNone (events.js:85:20)
    at IncomingMessage.emit (events.js:179:7)
    at endReadableNT (_stream_readable.js:913:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
working/usr/local/lib/node_modules/slack-history-export/node_modules/slack-api-client/lib/api/common/base_class.js:40
            throw err;

Without knowing much Node I suspect that maybe the first few requests (auth, find user-id, etc.) don't check the rate-limiting? Once it starts downloading messages it seems to work fine.

$ slack-history-export -V
0.4.1
hisabimbola commented 7 years ago

Thanks for reporting, I will look into this...

ste00martin commented 7 years ago

I also got:

Error: Too Many Requests
    at Request.callback (/usr/local/lib/node_modules/slack-history-export/node_modules/superagent/lib/node/index.js:698:17)
    at IncomingMessage.<anonymous> (/usr/local/lib/node_modules/slack-history-export/node_modules/superagent/lib/node/index.js:922:12)
    at emitNone (events.js:91:20)
    at IncomingMessage.emit (events.js:188:7)
    at endReadableNT (_stream_readable.js:974:12)
    at _combinedTickCallback (internal/process/next_tick.js:80:11)
    at process._tickCallback (internal/process/next_tick.js:104:9)

after running

slack-history-export --token "token" --channel "#all"
humor4fun commented 7 years ago

I might have to build in rate limiting into my own script since I use yours too.

humor4fun commented 7 years ago

You just need to tone down to one request per second, then it will be perfectly fine.

https://api.slack.com/docs/rate-limits#outgoing_webhooks

rdavison commented 7 years ago

I created a pull request to address this issue: https://github.com/hisabimbola/slack-history-export/pull/47/files

hisabimbola commented 6 years ago

I just tested this and I do not seem to be getting rate limiting issue. This is because the slack sdk module being used implemented some retry mechanism as seen here.

Closing this issue

501st-alpha1 commented 6 years ago

@hisabimbola Unfortunately, I don't think this is actually working. I've been getting Error Too Many Requests (HTTP 429) when using slack-backup to back up my entire workspace at once.

Looking at the package.json for slack-history-export, I see you are set to use slack 8.1.2. The closest tag I could find one GitHub is 8.4.2, and this is _exec.js at v8.4.2. That version doesn't show any rate limiting code except for a commented-out variable.

(Aside: in your last comment you linked to master rather than a specific commit/tag, so it's possible that the code changed in the meantime. If you want to avoid that, in the GitHub UI you can click the dropdown at top-left to select a specific tag, or click the commit link at top right. You can also just replace master in the URL with the tag or commit SHA that you want to use.)

Additionally, this is the rate-limiting code in the current version of the file you linked to:

if (err && err.message === 'POST failed with: 429') {
  // workaround Slacks lack of symmetry not ours…
  var e = Error('ratelimited')
  e.retry = err.raw.headers['retry-after']
  callback(e)
}

so it looks like it only builds an error to tell you that you've been rate-limited, it doesn't do any automatic retrying.

In order to fix this, I think you'd need to update to the latest version of the slack package and then add some retry code for each call made. (E.g. here for the users list.)

I'd offer to write a PR myself, but I can't guarantee I'll be able to commit the time anytime soon, and I can work around this for now by manually re-downloading any failed files.