From 0c8a4b6e566dc257c529c23874620ae8485b8f39 Mon Sep 17 00:00:00 2001 From: Jatin Dhankhar Date: Tue, 30 Jun 2020 20:44:19 +0530 Subject: [PATCH 1/4] Add context with rate limit struct if class belongs to Octokit::Forbidden --- lib/octokit/error.rb | 5 ++++- lib/octokit/rate_limit.rb | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/octokit/error.rb b/lib/octokit/error.rb index cf53245ca..80259340b 100644 --- a/lib/octokit/error.rb +++ b/lib/octokit/error.rb @@ -34,9 +34,12 @@ def self.from_response(response) end end - def initialize(response=nil) + def initialize(response=nil,context=nil) @response = response super(build_error_message) + if self.class.superclass == Octokit::Forbidden + context = Octokit::RateLimit.from_response(response) + end end # Documentation URL returned by the API for some errors diff --git a/lib/octokit/rate_limit.rb b/lib/octokit/rate_limit.rb index b6ff0fb37..201bc674c 100644 --- a/lib/octokit/rate_limit.rb +++ b/lib/octokit/rate_limit.rb @@ -20,7 +20,7 @@ class RateLimit < Struct.new(:limit, :remaining, :resets_at, :resets_in) # @return [RateLimit] def self.from_response(response) info = new - if response && !response.headers.nil? + if response && response.respond_to?(:headers) && !response.headers.nil? info.limit = (response.headers['X-RateLimit-Limit'] || 1).to_i info.remaining = (response.headers['X-RateLimit-Remaining'] || 1).to_i info.resets_at = Time.at((response.headers['X-RateLimit-Reset'] || Time.now).to_i) From ff0387a045703a2fafa7dab1d1083223f1f950bc Mon Sep 17 00:00:00 2001 From: Jatin Dhankhar Date: Fri, 3 Jul 2020 00:05:18 +0530 Subject: [PATCH 2/4] Add context builder method. Write demo spec --- lib/octokit/error.rb | 15 ++++++++++----- spec/octokit/client_spec.rb | 10 ++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/octokit/error.rb b/lib/octokit/error.rb index 80259340b..aa4118350 100644 --- a/lib/octokit/error.rb +++ b/lib/octokit/error.rb @@ -1,7 +1,7 @@ module Octokit # Custom error class for rescuing from all GitHub errors class Error < StandardError - + attr_reader :context # Returns the appropriate Octokit::Error subclass based # on status and response message # @@ -34,12 +34,17 @@ def self.from_response(response) end end - def initialize(response=nil,context=nil) + def build_error_context + rate_limited_errors = [Octokit::TooManyRequests, Octokit::AbuseDetected] + if rate_limited_errors.include?(self.class) + @context = Octokit::RateLimit.from_response(@response) + end + end + + def initialize(response=nil) @response = response super(build_error_message) - if self.class.superclass == Octokit::Forbidden - context = Octokit::RateLimit.from_response(response) - end + build_error_context end # Documentation URL returned by the API for some errors diff --git a/spec/octokit/client_spec.rb b/spec/octokit/client_spec.rb index de4a5234c..77f3d2ec7 100644 --- a/spec/octokit/client_spec.rb +++ b/spec/octokit/client_spec.rb @@ -1055,6 +1055,16 @@ end end + it "returns empty context when non rate limit error occurs" do + stub_get('/user').to_return \ + :status => 509, + :headers => { + :content_type => "application/json", + }, + :body => {:message => "Bandwidth exceeded"}.to_json + expect { Octokit.get('/user') }.to raise_error(an_instance_of(Octokit::ServerError).and having_attributes({context: nil})) + end + describe "module call shortcut" do before do Octokit.reset! From 8977c030dcee52be88c30429f063296e6e71da01 Mon Sep 17 00:00:00 2001 From: Jatin Dhankhar Date: Fri, 3 Jul 2020 23:31:27 +0530 Subject: [PATCH 3/4] Add correct specs --- spec/octokit/client_spec.rb | 39 ++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/spec/octokit/client_spec.rb b/spec/octokit/client_spec.rb index 77f3d2ec7..7aadb4d93 100644 --- a/spec/octokit/client_spec.rb +++ b/spec/octokit/client_spec.rb @@ -1062,7 +1062,44 @@ :content_type => "application/json", }, :body => {:message => "Bandwidth exceeded"}.to_json - expect { Octokit.get('/user') }.to raise_error(an_instance_of(Octokit::ServerError).and having_attributes({context: nil})) + begin + Octokit.get('/user') + rescue Octokit::ServerError => server_error + expect(server_error.context).to be_nil + end + end + + it "returns context with default data when rate limit error occurs but headers are missing" do + stub_get('/user').to_return \ + :status => 403, + :headers => { + :content_type => "application/json", + }, + :body => {:message => "rate limit exceeded"}.to_json + begin + expect_any_instance_of(Faraday::Env).to receive(:headers).at_least(:once).and_return({}) + Octokit.get('/user') + rescue Octokit::TooManyRequests => rate_limit_error + expect(rate_limit_error.context).to be_an_instance_of(Octokit::RateLimit) + end + end + + it "returns context when non rate limit error occurs but rate limit headers are present" do + stub_get('/user').to_return \ + :status => 403, + :headers => { + 'content_type' => 'application/json' + }, + :body => {:message => "rate limit exceeded"}.to_json + begin + rate_limit_headers = {'X-RateLimit-Limit' => 60, 'X-RateLimit-Remaining' => 42, 'X-RateLimit-Reset' => (Time.now + 60).to_i} + expect_any_instance_of(Faraday::Env).to receive(:headers).at_least(:once).and_return(rate_limit_headers) + Octokit.get('/user') + rescue Octokit::TooManyRequests => rate_limit_error + expect(rate_limit_error.context).to be_an_instance_of(Octokit::RateLimit) + expect(rate_limit_error.context.limit).to eql 60 + expect(rate_limit_error.context.remaining).to eql 42 + end end describe "module call shortcut" do From 70f6ba6bef59c359f41d9f746976295be7c670cd Mon Sep 17 00:00:00 2001 From: Jatin Dhankhar Date: Mon, 6 Jul 2020 23:47:03 +0530 Subject: [PATCH 4/4] Refactor rate limited errors to a constant --- lib/octokit/error.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/octokit/error.rb b/lib/octokit/error.rb index aa4118350..121f96b12 100644 --- a/lib/octokit/error.rb +++ b/lib/octokit/error.rb @@ -35,8 +35,7 @@ def self.from_response(response) end def build_error_context - rate_limited_errors = [Octokit::TooManyRequests, Octokit::AbuseDetected] - if rate_limited_errors.include?(self.class) + if RATE_LIMITED_ERRORS.include?(self.class) @context = Octokit::RateLimit.from_response(@response) end end @@ -323,4 +322,5 @@ class ApplicationCredentialsRequired < StandardError; end # Raised when a repository is created with an invalid format class InvalidRepository < ArgumentError; end + RATE_LIMITED_ERRORS = [Octokit::TooManyRequests, Octokit::AbuseDetected] end