reimplement error handling

lib/instapaper/error.rb

@@ -4,40 +4,69 @@ module Instapaper
     # @return [Integer]
     attr_reader :code
 
-#     RateLimitExceeded = Class.new(self)
+    BookmarkError = Class.new(self)
-#     PremiumAccountRequired = Class.new(self)
+    FolderError = Class.new(self)
-#     SuspendedApplication = Class.new(self)
+    HighlightError = Class.new(self)
-#
-# # 1040: Rate-limit exceeded
-# # 1041: Premium account required
-# # 1042: Application is suspended
-#
-#     BookmarkError = Class.new(self)
-#
-# Bookmark errors:
-#
-# 1220: Domain requires full content to be supplied
-# 1221: Domain has opted out of Instapaper compatibility
-# 1240: Invalid URL specified
-# 1241: Invalid or missing bookmark_id
-# 1242: Invalid or missing folder_id
-# 1243: Invalid or missing progress
-# 1244: Invalid or missing progress_timestamp
-# 1245: Private bookmarks require supplied content
-# 1250: Unexpected error when saving bookmark
-# Folder errors:
-#
-# 1250: Invalid or missing title
-# 1251: User already has a folder with this title
-# 1252: Cannot add bookmarks to this folder
-# Operational errors:
-#
-# 1500: Unexpected service error
-# 1550: Error generating text version of this URL
-# Highlight Errors:
-#
-# 1600: Cannot create highlight with empty text
-# 1601: Duplicate highlight
 
+    ERRORS = {
+      1040 => 'Rate-limit exceeded',
+      1041 => 'Premium account required',
+      1042 => 'Application is suspended',
+      1500 => 'Unexpected service error',
+      1550 => 'Error generating text version of this URL',
+    }
+
+    BOOKMARK_ERRORS = {
+      1220 => 'Domain requires full content to be supplied',
+      1221 => 'Domain has opted out of Instapaper compatibility',
+      1240 => 'Invalid URL specified',
+      1241 => 'Invalid or missing bookmark_id',
+      1242 => 'Invalid or missing folder_id',
+      1243 => 'Invalid or missing progress',
+      1244 => 'Invalid or missing progress_timestamp',
+      1245 => 'Private bookmarks require supplied content',
+      1250 => 'Unexpected error when saving bookmark',
+    }
+
+    FOLDER_ERRORS = {
+      1250 => 'Invalid or missing title',
+      1251 => 'User already has a folder with this title',
+      1252 => 'Cannot add bookmarks to this folder',
+    }
+
+    HIGHLIGHT_ERRORS = {
+      1600 => 'Cannot create highlight with empty text',
+      1601 => 'Duplicate highlight',
+    }
+
+    CODES = [
+      ERRORS,
+      BOOKMARK_ERRORS,
+      FOLDER_ERRORS,
+      HIGHLIGHT_ERRORS,
+    ].collect { |e| e.keys }.flatten
+
+    # Create a new error from an HTTP response
+    #
+    # @param response [HTTP::Response]
+    # @return [Instapaper::Error]
+    def self.from_response(code, path)
+      case path
+      when /highlights/ then HighlightError.new(HIGHLIGHT_ERRORS[code], code)
+      when /bookmarks/ then BookmarkError.new(BOOKMARK_ERRORS[code], code)
+      when /folders/ then FolderError.new(FOLDER_ERRORS[code], code)
+      else new(ERRORS[code], code)
+      end
+    end
+
+    # Initializes a new Error object
+    #
+    # @param message [Exception, String]
+    # @param code [Integer]
+    # @return [Instapaper::Error]
+    def initialize(message = '', code = nil)
+      super(message)
+      @code = code
+    end
   end
 end

lib/instapaper/http/request.rb

@@ -39,33 +39,18 @@ module Instapaper
         @headers = Instapaper::HTTP::Headers.new(@client, @request_method, @uri, @options).request_headers
         options_key = @request_method == :get ? :params : :form
         response = ::HTTP.with(@headers).public_send(@request_method, @uri.to_s, options_key => @options)
-        response_body = raw ? response.to_s : symbolize_keys!(response.parse)
+        fail_if_error(response)
-        response_headers = response.headers
+        raw ? response.to_s : symbolize_keys!(response.parse)
-        fail_or_return_response_body(response.code, response_body, response_headers)
       end
 
-      def fail_or_return_response_body(code, body, headers)
+      def fail_if_error(response)
-        error = nil # error(code, body, headers)
+        error = error(response.code)
         fail(error) if error
-        body
       end
 
-      def error(code, body, headers)
+      def error(code)
-        klass = Instapaper::Error::ERRORS[code]
+        if Instapaper::Error::CODES.index(code.to_i)
-        if klass == Instapaper::Error::Forbidden
+          Instapaper::Error.from_response(code, @path)
-          forbidden_error(body, headers)
-        elsif !klass.nil?
-          klass.from_response(body, headers)
-        end
-      end
-
-      def forbidden_error(body, headers)
-        error = Instapaper::Error::Forbidden.from_response(body, headers)
-        klass = Instapaper::Error::FORBIDDEN_MESSAGES[error.message]
-        if klass
-          klass.from_response(body, headers)
-        else
-          error
         end
       end
 

spec/instapaper/error_spec.rb

@@ -0,0 +1,69 @@
+require 'spec_helper'
+
+describe Instapaper::Error do
+  before do
+    @client = Instapaper::Client.new(consumer_key: 'CK', consumer_secret: 'CS', access_token: 'AT', access_token_secret: 'AS')
+  end
+
+  describe '#code' do
+    it 'returns the error code' do
+      error = Instapaper::Error.new('execution expired', 123)
+      expect(error.code).to eq(123)
+    end
+  end
+
+  describe '#message' do
+    it 'returns the error message' do
+      error = Instapaper::Error.new('execution expired')
+      expect(error.message).to eq('execution expired')
+    end
+  end
+
+  Instapaper::Error::ERRORS.each do |status, exception|
+    context "when HTTP status is #{status}" do
+      before do
+        stub_post('/api/1/oauth/access_token')
+          .to_return(status: status, body: '', headers: {content_type: 'application/json; charset=utf-8'})
+      end
+      it "raises #{exception}" do
+        expect { @client.token('foo', 'bar') }.to raise_error(Instapaper::Error)
+      end
+    end
+  end
+
+  Instapaper::Error::BOOKMARK_ERRORS.each do |status, exception|
+    context "when HTTP status is #{status}" do
+      before do
+        stub_post('/api/1/bookmarks/list')
+          .to_return(status: status, body: '', headers: {content_type: 'application/json; charset=utf-8'})
+      end
+      it "raises #{exception}" do
+        expect { @client.bookmarks }.to raise_error(Instapaper::Error::BookmarkError)
+      end
+    end
+  end
+
+  Instapaper::Error::FOLDER_ERRORS.each do |status, exception|
+    context "when HTTP status is #{status}" do
+      before do
+        stub_post('/api/1/folders/list')
+          .to_return(status: status, body: '', headers: {content_type: 'application/json; charset=utf-8'})
+      end
+      it "raises #{exception}" do
+        expect { @client.folders }.to raise_error(Instapaper::Error::FolderError)
+      end
+    end
+  end
+
+  Instapaper::Error::HIGHLIGHT_ERRORS.each do |status, exception|
+    context "when HTTP status is #{status}" do
+      before do
+        stub_post('/api/1.1/bookmarks/123/highlights')
+          .to_return(status: status, body: fixture('highlights_list.json'), headers: {content_type: 'application/json; charset=utf-8'})
+      end
+      it "raises #{exception}" do
+        expect { @client.highlights('123') }.to raise_error(Instapaper::Error::HighlightError)
+      end
+    end
+  end
+end