maknz / slack

A simple PHP package for sending messages to Slack, with a focus on ease of use and elegant syntax.
BSD 2-Clause "Simplified" License
1.17k stars 204 forks source link

Doesn't handle Slacks rate limits. #36

Closed zechdc closed 8 years ago

zechdc commented 8 years ago

Ran into an interesting bug tonight. I received an email from slack that said the following:

Hi there, We're writing to let you know that one of your Incoming-Webhook integrations was automatically disabled on February 16, 2016 at 08:52 PM CST. It was sending a lot of messages – over 3,600 in the 5 minutes directly before it was disabled.

Since Slack is primarily a tool for humans to communicate with one another, this is a preventative measure to make sure that an out-of-control script doesn't compromise the quality of your archive or make it difficult for you to talk to one another.

To prevent this from happening in the future, you'll want to implement error checking in your integration. When we detect that one of your integrations might be running out of control (current thresholds are more than one message per second over a sustained period of time), we'll return an HTTP 429 and a chunk of JSON with some more details.

  {
      "ok": false,
      "count_hour_ago": 400,
      "count_minute_ago": 100,
      "count_second_ago": 5
  }

When we send one of these error messages back, it also means that your message did not make it into Slack – you'll want to queue it up and try again in a few seconds. You'll find more details about our rate limits here:

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

Our system had a hardware issue and linux went into read only mode causing thousands of exceptions to be thrown.

After the first thousand exceptions got logged, I started getting a few of these scattered in my log 'Uncaught exception with message 'Client error response [url] https://hooks.slack.com/services/REDACTED_MY_KEY/REDACTED_MY_KEY [status code] 429 [reason phrase] Too Many Requests'

maknz commented 8 years ago

This isn't a bug. Slack informed you of the issue, and the package gave you a sensible exception explaining the issue when you attempted to send a message. How would you suggest this be handled?

zechdc commented 8 years ago

This is an edge case for sure. The system does handle itself pretty well. All I had to do was enable the disabled webhook.

I guess I would expect your PHP package to see that slack returned an HTTP 429 and start sending the response of the 429 every second to abide by slacks rate limits. However, that would probably be a decent amount of work for something that works well enough. Just figured I'd inform you of an edge case. Thanks for the reply.

Thanks, Zechariah Campbell | Band Manager | JanuaryMay.com http://januarymay.com/ | 314-707-0952

On Wed, Feb 17, 2016 at 1:42 PM, Regan McEntyre notifications@github.com wrote:

Slack informed you of the issue, and the package gave you a sensible exception explaining the issue when you attempted to use the package. How would you suggest this be handled?

— Reply to this email directly or view it on GitHub https://github.com/maknz/slack/issues/36#issuecomment-185371532.

maknz commented 8 years ago

Cool. At this stage I'm happy with the existing behaviour. As with anything, if a decent chunk of people chime in with this being problematic for whatever reason, we can re-examine.