Merge pull request #50 from contentful/error_handling

Add a better error handling for the 503 error

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Change Log
 ## Unreleased
+### Fixed
+* Better handling of 503 responses from the API [#48](https://github.com/contentful/contentful-management.rb/pull/48)
+
 ### Added
 * `focus` and `fit` to image handling parameters [#44](https://github.com/contentful/contentful.rb/pull/44)
 

lib/contentful/resource_builder.rb

@@ -12,7 +12,7 @@
 
 module Contentful
   # Transforms a Contentful::Response into a Contentful::Resource or a Contentful::Error
-  # See example/resource_mapping.rb for avanced usage
+  # See example/resource_mapping.rb for advanced usage
   class ResourceBuilder
     DEFAULT_RESOURCE_MAPPING = {
       'Space' => Space,

lib/contentful/response.rb

@@ -31,8 +31,10 @@ def initialize(raw, request = nil)
         parse_json!
       elsif no_content_response?
         @status = :no_content
-      elsif no_resource_or_bad_request?
+      elsif invalid_response?
         parse_contentful_error
+      elsif service_unavailable_response?
+        service_unavailable_error
       else
         parse_http_error
       end
@@ -54,12 +56,22 @@ def valid_http_response?
       [200, 201].include?(raw.status)
     end
 
+    def service_unavailable_response?
+      @raw.status == 503
+    end
+
+    def service_unavailable_error
+      @status = :error
+      @error_message = 'Service Unavailable, contenful.com API seems to be down'
+      @object = Error[@raw.status].new(self)
+    end
+
     def parse_http_error
       @status = :error
       @object = Error[raw.status].new(self)
     end
 
-    def no_resource_or_bad_request?
+    def invalid_response?
       [400, 404].include?(raw.status)
     end
 

spec/array_spec.rb

@@ -63,7 +63,7 @@
     end
 
     it 'will return false if #request not available' do
-      expect(Contentful::Array.new({}).reload).to be_false
+      expect(Contentful::Array.new({}).reload).to be_falsey
     end
   end
 end

spec/error_class_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Contentful::Error do
-  let(:r) { Contentful::Response.new raw_fixture('not_found',404) }
+  let(:r) { Contentful::Response.new raw_fixture('not_found', 404) }
 
   describe '#response' do
     it 'returns the response the error has been initialized with' do
@@ -28,26 +28,31 @@
   end
 
   describe '.[]' do
-    it 'returns NotFound error class for 404' do
-      expect(Contentful::Error[404]).to eq Contentful::NotFound
-    end
 
     it 'returns BadRequest error class for 400' do
       expect(Contentful::Error[400]).to eq Contentful::BadRequest
     end
 
+    it 'returns Unauthorized error class for 401' do
+      expect(Contentful::Error[401]).to eq Contentful::Unauthorized
+    end
+
     it 'returns AccessDenied error class for 403' do
       expect(Contentful::Error[403]).to eq Contentful::AccessDenied
     end
 
-    it 'returns Unauthorized error class for 401' do
-      expect(Contentful::Error[401]).to eq Contentful::Unauthorized
+    it 'returns NotFound error class for 404' do
+      expect(Contentful::Error[404]).to eq Contentful::NotFound
     end
 
     it 'returns ServerError error class for 500' do
       expect(Contentful::Error[500]).to eq Contentful::ServerError
     end
 
+    it 'returns ServiceUnavailable error class for 503' do
+      expect(Contentful::Error[503]).to eq Contentful::ServiceUnavailable
+    end
+
     it 'returns generic error class for any other value' do
       expect(Contentful::Error[nil]).to eq Contentful::Error
       expect(Contentful::Error[200]).to eq Contentful::Error

spec/error_requests_spec.rb

@@ -14,7 +14,7 @@
   end
 
   it 'will return 403 (AccessDenied) if ...' do
-    pending
+    skip
   end
 
   it 'will return 401 (Unauthorized) if wrong credentials given' do
@@ -26,6 +26,14 @@
   end
 
   it 'will return 500 (ServerError) if ...' do
-    pending
+    skip
+  end
+
+  it 'will return 503 (ServiceUnavailable) if connection time out' do
+    client = Contentful::Client.new(space: 'wrong', access_token: 'credentials')
+
+    expect_vcr('unavailable'){
+      client.entry 'nyancat'
+    }.to raise_error(Contentful::ServiceUnavailable)
   end
 end

spec/field_spec.rb

@@ -26,11 +26,11 @@
     end
 
     it 'has #required' do
-      expect(field.required).to be_true
+      expect(field.required).to be_truthy
     end
 
     it 'has #localized' do
-      expect(field.required).to be_true
+      expect(field.required).to be_truthy
     end
   end
 end

spec/resource_spec.rb

@@ -46,7 +46,7 @@
     end
 
     it 'will return false if #request not available' do
-      expect(Contentful::ContentType.new({}).reload).to be_false
+      expect(Contentful::ContentType.new({}).reload).to be_falsey
     end
   end
 end

spec/sync_page_spec.rb

@@ -82,21 +82,21 @@
 
   describe '#next_page?' do
     it 'will return true if there is a next page' do
-      expect(page_with_more.next_page?).to be_true
+      expect(page_with_more.next_page?).to be_truthy
     end
 
     it 'will return false if last page' do
-      expect(page.next_page?).to be_false
+      expect(page.next_page?).to be_falsey
     end
   end
 
   describe '#last_page?' do
     it 'will return true if no more pages available' do
-      expect(page.last_page?).to be_true
+      expect(page.last_page?).to be_truthy
     end
 
     it 'will return false if more pages available' do
-      expect(page_with_more.last_page?).to be_false
+      expect(page_with_more.last_page?).to be_falsey
     end
   end
 

spec/sync_spec.rb

@@ -63,11 +63,11 @@
 
   describe '#completed?' do
     it 'will return true if no more pages to request in the current sync' do
-      expect(first_page.next_sync_url).to be_false
+      expect(first_page.next_sync_url).to be_falsey
     end
 
     it 'will return true if not all pages requested, yet' do
-      expect(last_page.next_sync_url).to be_true
+      expect(last_page.next_sync_url).to be_truthy
     end
   end