slack-ruby-client icon indicating copy to clipboard operation
slack-ruby-client copied to clipboard

Add option to disable raising exceptions on failed Slack responses or HTTP errors

Open mmplisskin opened this issue 8 years ago • 2 comments

I have noticed a lot of output responses that could be slightly more helpful. In this case ideally it would be nice to receive the error class or false. It seems like a lot of times in this gem exceptions are also raised unnecessarily when a true or false response could be given. screen shot 2017-11-14 at 4 49 41 pm

Here is another case where there could be some confusion as well and a standard language idiom could be followed.

screen shot 2017-11-14 at 4 54 58 pm

mmplisskin avatar Nov 15 '17 00:11 mmplisskin

The reason this kind of stuff matters is because if we want to have a simple method to check if the team is authorized we need to write exception handling i.e.:

instead of this

return Slack::Web::Client.new(token: token).auth_test.ok == true 

or this

return Slack::Web::Client.new(token: token).auth_test.success?

this would be required along with some additional memory for the exception overhead.

    begin
      return Slack::Web::Client.new(token: token).auth_test.ok == true
    rescue Slack::Web::Api::Errors::SlackError
      return false
    end

mmplisskin avatar Nov 15 '17 01:11 mmplisskin

This library deliberately raises exceptions for all errors by design, so you don't have to be checking the nagging ok field in responses which litters everyone's code. This is implemented in https://github.com/slack-ruby/slack-ruby-client/blob/master/lib/slack/web/faraday/response/raise_error.rb and used in https://github.com/slack-ruby/slack-ruby-client/blob/385b30b7db7dde2befd137f12841218cb957f82d/lib/slack/web/faraday/connection.rb#L25.

I think it would be useful to have it both ways, please feel free to PR options that will let users swap out both raising errors on HTTP statuses 400-600 and Slack responses.

dblock avatar Nov 15 '17 10:11 dblock