Improve 5XX error handling. (#41)

* Improve 5XX error handling. * Maybe fix travis builds. * Maybe fix travis builds. * Maybe fix Appveyor builds. * Maybe fix Appveyor builds. * Maybe fix Appveyor builds.
percy · Jan 31, 2019 · 28fba8e · 28fba8e
1 parent 80d806a
commit 28fba8e
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 16 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -8,7 +8,6 @@ addons:
     - libgmp-dev
 cache: bundler
 rvm:
-  - ruby-2.2.6
   - ruby-2.3.3
   - ruby-2.4.0
   - ruby-2.5.0

diff --git a/appveyor.yml b/appveyor.yml
@@ -3,12 +3,11 @@ environment:
     - RUBY_VERSION: 25
     - RUBY_VERSION: 24
     - RUBY_VERSION: 23
-    - RUBY_VERSION: 22
-    - RUBY_VERSION: 21
 
 # Install scripts. (runs after repo cloning)
 install:
   - set PATH=C:\Ruby%RUBY_VERSION%\bin;%PATH%
+  - gem install bundler
   - bundle install
 
 before_test:

diff --git a/lib/percy/client.rb b/lib/percy/client.rb
@@ -65,6 +65,12 @@ class BadGatewayError < ServerError; end
     # 503
     class ServiceUnavailableError < ServerError; end
 
+    # 504
+    class GatewayTimeoutError < ServerError; end
+
+    # 520..530.
+    class CloudflareError < ServerError; end
+
     attr_reader :config, :client_info, :environment_info
 
     def initialize(options = {})

diff --git a/lib/percy/client/connection.rb b/lib/percy/client/connection.rb
@@ -30,6 +30,10 @@ def on_complete(env)
             error_class = Percy::Client::BadGatewayError
           when 503
             error_class = Percy::Client::ServiceUnavailableError
+          when 504
+            error_class = Percy::Client::GatewayTimeoutError
+          when 520..530
+            error_class = Percy::Client::CloudflareError
           when CLIENT_ERROR_STATUS_RANGE # Catchall.
             error_class = Percy::Client::HttpError
           end
@@ -70,9 +74,9 @@ def get(path, options = {})
           raise Percy::Client::TimeoutError
         rescue Faraday::ConnectionFailed
           raise Percy::Client::ConnectionFailed
-        rescue Percy::Client::HttpError => e
-          # Retry on 502 errors.
-          if e.status == 502 && (retries -= 1) >= 0
+        rescue Percy::Client::ServerError => e
+          # Retry on 5XX errors.
+          if (retries -= 1) >= 0
             sleep(rand(1..3))
             retry
           end
@@ -98,6 +102,7 @@ def post(path, data, options = {})
         rescue Faraday::ConnectionFailed
           raise Percy::Client::ConnectionFailed
         rescue Percy::Client::ServerError => e
+          # Retry on 5XX errors.
           if (retries -= 1) >= 0
             sleep(rand(1..3))
             retry

diff --git a/percy-client.gemspec b/percy-client.gemspec
@@ -23,7 +23,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'excon'
   spec.add_dependency 'addressable'
 
-  spec.add_development_dependency 'bundler', '~> 1.7'
+  spec.add_development_dependency 'bundler', '~> 2.0'
   spec.add_development_dependency 'rake', '~> 10.0'
   spec.add_development_dependency 'rspec', '~> 3.2'
   spec.add_development_dependency 'vcr'

diff --git a/spec/lib/percy/client/connection_spec.rb b/spec/lib/percy/client/connection_spec.rb
@@ -12,6 +12,8 @@
   let(:ci_version) { '8.14.3-ee' }
   let(:uri) { "#{Percy.config.api_url}/test" }
 
+  SERVER_ERROR_CODES = ((500..504).to_a + (520..530).to_a).freeze
+
   shared_examples_for 'a connection that sets headers with HTTP method' do |http_method|
     it 'sets headers' do
       stub_request(http_method, uri)
@@ -51,12 +53,14 @@
       expect { response }.to raise_error(Percy::Client::ConnectionFailed)
     end
 
-    it 'retries on 502 errors' do
-      stub_request(:get, uri)
-        .to_return(body: {foo: true}.to_json, status: 502)
-        .then.to_return(body: {foo: true}.to_json, status: 200)
+    it 'retries on 5XX errors' do
+      SERVER_ERROR_CODES.each do |error_code|
+        stub_request(:get, uri)
+          .to_return(body: {foo: true}.to_json, status: error_code)
+          .then.to_return(body: {foo: true}.to_json, status: 200)
 
-      expect(response).to eq('foo' => true)
+        expect(response).to eq('foo' => true)
+      end
     end
 
     it 'raises error after 3 retries' do
@@ -124,18 +128,23 @@
       '500' => Percy::Client::InternalServerError,
       '502' => Percy::Client::BadGatewayError,
       '503' => Percy::Client::ServiceUnavailableError,
+      '504' => Percy::Client::GatewayTimeoutError,
+      '520' => Percy::Client::CloudflareError,
+      '530' => Percy::Client::CloudflareError,
     }
 
     http_errors.each do |http_status, error_class|
       include_examples 'HTTP status raises custom error class', http_status, error_class
     end
 
     it 'retries on server errors' do
-      stub_request(:post, uri)
-        .to_return(body: {foo: true}.to_json, status: 500)
-        .then.to_return(body: {foo: true}.to_json, status: 200)
+      SERVER_ERROR_CODES.each do |error_code|
+        stub_request(:post, uri)
+          .to_return(body: {foo: true}.to_json, status: 500)
+          .then.to_return(body: {foo: true}.to_json, status: 200)
 
-      expect(response).to eq('foo' => true)
+        expect(response).to eq('foo' => true)
+      end
     end
 
     it 'raises error after 3 retries' do