search

slack-ruby / slack-ruby-client

A Ruby and command-line client for the Slack Web, Real Time Messaging and Event APIs.
MIT License
1.21k stars 214 forks source link

Raise rescuable errors #417

Closed zachahn closed 2 years ago

zachahn commented 2 years ago

This commit replaces all instances of throw with raise.

While I was using this library, I noticed that error handling didn't work as expected.

Here's an example of what I tried. Looking at the code, I expected this to print ArgumentError, but it printed UncaughtThrowError.

client = Slack::Web::Client.new(token: ENV["TOKEN"])
begin
  client.chat_postMessage
rescue => e
  puts e.class
end
# UncaughtThrowError

According to Ruby's tutorial on Exceptions, raise and rescue should be used for exceptions, while throw and catch could be used for control flow.

dblock commented 2 years ago

Fair enough, thanks!

Please add a line to CHANGELOG, get CI to pass, there's a bunch of Rubocop suggestions.

zachahn commented 2 years ago

Done and done. Since I modified some patch files, I verified my changes by running bundle exec rake slack:api:update (I didn't commit those changes).

Let me know if there's anything else is missing. Thanks!

dblock commented 2 years ago

Done and done. Since I modified some patch files, I verified my changes by running bundle exec rake slack:api:update (I didn't commit those changes).

~Looks like they are committed tho (those files that have "This file was auto-generated by lib/tasks/web.rake" on top). I would either include all those generated changes, or include none of the generated changes (and make a separate PR/commit with those)?~

I see you made all the throw/raise changes only, so I'll just merge this as is.

dblock commented 2 years ago

@zachahn Want to followup with other API updates and we can make a release?