Merge pull request #359 from agrobbin/500-html-response

dblock · web-flow · commit cddc8982bd14 · 2021-01-26T13:14:58.000-05:00
Handle non-JSON 500 errors
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ### 0.16.1 (Next)
 
 * Your contribution here.
+* [#359](https://github.com/slack-ruby/slack-ruby-client/pull/359): Handle non-JSON 500 errors - [@agrobbin](https://github.com/agrobbin).
 
 ### 0.16.0 (2021/01/24)
 
diff --git a/lib/slack/web/faraday/connection.rb b/lib/slack/web/faraday/connection.rb
@@ -24,10 +24,10 @@ def connection
               ::Faraday::Connection.new(endpoint, options) do |connection|
                 connection.use ::Faraday::Request::Multipart
                 connection.use ::Faraday::Request::UrlEncoded
-                connection.use ::Slack::Web::Faraday::Response::WrapError
                 connection.use ::Slack::Web::Faraday::Response::RaiseError
                 connection.use ::FaradayMiddleware::Mashify, mash_class: Slack::Messages::Message
                 connection.use ::FaradayMiddleware::ParseJson
+                connection.use ::Slack::Web::Faraday::Response::WrapError
                 connection.response :logger, logger if logger
                 connection.adapter adapter
               end
diff --git a/lib/slack/web/faraday/response/raise_error.rb b/lib/slack/web/faraday/response/raise_error.rb
@@ -23,12 +23,8 @@ def on_complete(env)
 
           def call(env)
             super
-          rescue Slack::Web::Api::Errors::SlackError, Slack::Web::Api::Errors::TooManyRequestsError
-            raise
           rescue ::Faraday::ParsingError
             raise Slack::Web::Api::Errors::ParsingError.new('parsing_error', env.response)
-          rescue ::Faraday::TimeoutError, ::Faraday::ConnectionFailed
-            raise Slack::Web::Api::Errors::TimeoutError.new('timeout_error', env.response)
           end
         end
       end
diff --git a/lib/slack/web/faraday/response/wrap_error.rb b/lib/slack/web/faraday/response/wrap_error.rb
@@ -3,14 +3,20 @@ module Slack
   module Web
     module Faraday
       module Response
-        class WrapError < ::Faraday::Response::RaiseError
+        class WrapError < ::Faraday::Response::Middleware
+          UNAVAILABLE_ERROR_STATUSES = (500..599).freeze
+
           def on_complete(env)
-            super
-          rescue Slack::Web::Api::Errors::SlackError
-            raise
-          rescue ::Faraday::ServerError
+            return unless UNAVAILABLE_ERROR_STATUSES.cover?(env.status)
+
             raise Slack::Web::Api::Errors::UnavailableError.new('unavailable_error', env.response)
           end
+
+          def call(env)
+            super
+          rescue ::Faraday::TimeoutError, ::Faraday::ConnectionFailed
+            raise Slack::Web::Api::Errors::TimeoutError.new('timeout_error', env.response)
+          end
         end
       end
     end
diff --git a/spec/slack/web/client_spec.rb b/spec/slack/web/client_spec.rb
@@ -322,12 +322,22 @@
       end
 
       context '5xx response' do
-        before { stub_slack_request.to_return(status: 500, body: '{}') }
+        context 'with a JSON body' do
+          before { stub_slack_request.to_return(status: 500, body: '{}') }
 
-        it 'raises UnavailableError' do
-          expect { request }.to raise_error(Slack::Web::Api::Errors::UnavailableError).with_message('unavailable_error')
-          expect(exception.cause).to be_a(Faraday::ServerError)
-          expect(exception.response.status).to eq(500)
+          it 'raises UnavailableError' do
+            expect { request }.to raise_error(Slack::Web::Api::Errors::UnavailableError).with_message('unavailable_error')
+            expect(exception.response.status).to eq(500)
+          end
+        end
+
+        context 'with a HTML response' do
+          before { stub_slack_request.to_return(status: 500, body: '<html></html>') }
+
+          it 'raises UnavailableError' do
+            expect { request }.to raise_error(Slack::Web::Api::Errors::UnavailableError).with_message('unavailable_error')
+            expect(exception.response.status).to eq(500)
+          end
         end
       end
     end
