Raise a dedicated error class with 429 responses#154
Raise a dedicated error class with 429 responses#154dblock merged 2 commits intoslack-ruby:masterfrom
Conversation
| it 'fails with an specific exception' do | ||
| begin | ||
| client.auth_test | ||
| rescue Slack::Web::Api::TooManyRequestsError => e |
There was a problem hiding this comment.
Note that this new error class does not inherit from Faraday::ClientError, so if a client is expecting to rescue that error in the case of a 429 response code, this PR will break that behavior.
greggroth
force-pushed
the
429-errors
branch
2 times, most recently
from
b07a326 to
dd0e69e
Compare
Ensure the error message includes the value of the "Retry-After" header so the client knows when it's safe to retry requests.
| end | ||
| require_relative 'slack/web/config' | ||
| require_relative 'slack/web/api/error' | ||
| require_relative 'slack/web/api/too_many_requests_error' |
There was a problem hiding this comment.
Lets move it to an errors folder?
| module Slack | ||
| module Web | ||
| module Api | ||
| class TooManyRequestsError < ::Faraday::Error |
There was a problem hiding this comment.
It may be confusing that this error is namespaced under Slack::Web::Api::Errors while the existing Slack::Web::Api::Error class is not namespaced (and is very close to Slack::Web::Api::Errors). Do you still want the change?
There was a problem hiding this comment.
I think so. It's not uncommon. We can also do a Slack::Web::Api::Errors::Base and alias the old one for backward compatibility if you want to, your call.
|
This is great, just move it into a folder structure for future errors and I'll merge. |
I moved `Slack::Web::Api::Error` to `Slack::Web::Api::Errors::SlackError` but left the orginal constant aliased to the new one.
|
Looks great, merging. |
2 participants
Ensure the error message includes the value of the "Retry-After" header
so the client knows when it's safe to retry requests.
Closes #153