From 918dadb5f019fafba7784a9e34fdeb184f323672 Mon Sep 17 00:00:00 2001 From: Shane Cavanaugh Date: Sat, 17 Jun 2017 12:03:53 -0400 Subject: [PATCH 1/5] Subclass Error to handle Server and Client errors --- lib/tracker_api.rb | 2 ++ lib/tracker_api/client.rb | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/tracker_api.rb b/lib/tracker_api.rb index 3191fec..9a4f850 100644 --- a/lib/tracker_api.rb +++ b/lib/tracker_api.rb @@ -29,6 +29,8 @@ module TrackerApi module Errors class UnexpectedData < StandardError; end + class ClientError < Error; end + class ServerError < Error; end end module Endpoints diff --git a/lib/tracker_api/client.rb b/lib/tracker_api/client.rb index c092451..d8dd04b 100644 --- a/lib/tracker_api/client.rb +++ b/lib/tracker_api/client.rb @@ -248,7 +248,11 @@ def request(method, options = {}) end response rescue Faraday::Error::ClientError => e - raise TrackerApi::Error.new(e) + case e.response[:status] + when 400..499 then raise TrackerApi::Errors::ClientError.new(e) + when 500..599 then raise TrackerApi::Errors::ServerError.new(e) + else raise TrackerApi::Error.new(e) + end end class Pagination From d27dce410add25b92511c3e2e04399a099b45cfe Mon Sep 17 00:00:00 2001 From: Shane Cavanaugh Date: Sat, 17 Jun 2017 15:47:41 -0400 Subject: [PATCH 2/5] Rescued error is expected to be for status codes 4xx and 5xx Faraday's ClientError is raised for status codes 400...600 so we'll never need to send the rescued exception straight to Error --- lib/tracker_api/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tracker_api/client.rb b/lib/tracker_api/client.rb index d8dd04b..f182e5a 100644 --- a/lib/tracker_api/client.rb +++ b/lib/tracker_api/client.rb @@ -251,7 +251,7 @@ def request(method, options = {}) case e.response[:status] when 400..499 then raise TrackerApi::Errors::ClientError.new(e) when 500..599 then raise TrackerApi::Errors::ServerError.new(e) - else raise TrackerApi::Error.new(e) + else raise "Expected 4xx or 5xx HTTP status code" end end From 73595d1ecd869b3ae85287bba88415a65ef0471b Mon Sep 17 00:00:00 2001 From: Shane Cavanaugh Date: Sun, 18 Jun 2017 12:22:52 -0400 Subject: [PATCH 3/5] Update README.md to include info about error handling --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 9d4e836..b806099 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,10 @@ client.project(project_id).stories(fields: ':default,tasks') # Eage story.comments(fields: ':default,person') # Eagerly get comments and the person that made the comment for a story ``` +## Error Handling +`TrackerApi::Errors::ClientError` is raised for 4xx HTTP status codes +`TrackerApi::Errors::ServerError` is raised for 5xx HTTP status codes + ## Warning Direct mutation of an attribute value skips coercion and dirty tracking. Please use direct assignment or the specialized add_* methods to get expected behavior. @@ -118,6 +122,7 @@ story.save - Add missing resources and endpoints - Add create, update, delete for resources +- Error handling for [error responses](https://www.pivotaltracker.com/help/api#Error_Responses) ## Semantic Versioning http://semver.org/ From b68f84fc29ee448bb2a3b7ca293795f27f65e7f6 Mon Sep 17 00:00:00 2001 From: Shane Cavanaugh Date: Sun, 18 Jun 2017 13:45:09 -0400 Subject: [PATCH 4/5] Test that ClientError and ServerError are raised for specific status codes --- test/error_test.rb | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 test/error_test.rb diff --git a/test/error_test.rb b/test/error_test.rb new file mode 100644 index 0000000..0d1b9de --- /dev/null +++ b/test/error_test.rb @@ -0,0 +1,32 @@ +require_relative 'minitest_helper' + +describe TrackerApi::Error do + let(:pt_user) { PT_USER_1 } + let(:client) { TrackerApi::Client.new token: pt_user[:token] } + let(:options) { { url: nil, headers: nil } } + + it 'raises ClientError for 4xx HTTP status codes' do + (400..499).each do |status_code| + mock_faraday_error(status_code) + assert_raises TrackerApi::Errors::ClientError do + client.send(:request, :get, options) + end + end + end + + it 'raises ServerError for 5xx HTTP status codes' do + (500..599).each do |status_code| + mock_faraday_error(status_code) + assert_raises TrackerApi::Errors::ServerError do + client.send(:request, :get, options) + end + end + end + + # Simulate the error Faraday will raise with a specific HTTP status code so + # we can test our rescuing of those errors + def mock_faraday_error(status_code) + ::Faraday::Connection.any_instance.stubs(:get). + raises(::Faraday::Error::ClientError.new(nil, { status: status_code})) + end +end From 848891688136c0e1110da5f4e7b9ae6bae7c9c7d Mon Sep 17 00:00:00 2001 From: Shane Cavanaugh Date: Sun, 18 Jun 2017 14:00:22 -0400 Subject: [PATCH 5/5] Test for RuntimeError we raise when status code < 400 and > 500 --- test/error_test.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/error_test.rb b/test/error_test.rb index 0d1b9de..ac46ddf 100644 --- a/test/error_test.rb +++ b/test/error_test.rb @@ -22,6 +22,15 @@ end end end + + it 'raises RuntimeError for HTTP status codes < 400 and > 500' do + [399, 600].each do |status_code| + mock_faraday_error(status_code) + assert_raises RuntimeError, "Expected 4xx or 5xx HTTP status code" do + client.send(:request, :get, options) + end + end + end # Simulate the error Faraday will raise with a specific HTTP status code so # we can test our rescuing of those errors