Merge branch 'retry-youtube-request-when-they-might-work-the-second-t…

…ime'

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    yt (0.5.12)
+    yt (0.5.13)
       activesupport
 
 GEM

HISTORY.md

@@ -19,6 +19,7 @@ v0.5 - 2014/05/16
 * New ContentOwner subclass of Account with access to partnered channels
 * Automatically refresh the access token when it expires or becomes invalid
 * Retry once YouTube earning queries that return error 503
+* Wait 3 seconds and retry *every* request that returns 500, 503 or 400 with "Invalid query"
 
 v0.4 - 2014/05/09
 --------------------

README.md

@@ -365,7 +365,7 @@ To install on your system, run
 
 To use inside a bundled Ruby project, add this line to the Gemfile:
 
-    gem 'yt', '~> 0.5.12'
+    gem 'yt', '~> 0.5.13'
 
 Since the gem follows [Semantic Versioning](http://semver.org),
 indicating the full version in your Gemfile (~> *major*.*minor*.*patch*)

lib/yt/collections/earnings.rb

@@ -4,17 +4,9 @@ module Yt
   module Collections
     class Earnings < Base
 
-      def within(days_range, try_again = true)
+      def within(days_range)
         @days_range = days_range
         Hash[*flat_map{|daily_earning| daily_earning}]
-      # NOTE: Once in a while, YouTube responds with 400 Error and the message
-      # "Invalid query. Query did not conform to the expectations."; in this
-      # case running the same query after one second fixes the issue. This is
-      # not documented by YouTube and hardly testable, but trying again the
-      # same query is a workaround that works and can hardly cause any damage.
-      # Similarly, once in while YouTube responds with a random 503 error.
-      rescue Yt::Error => e
-        try_again && rescue?(e) ? sleep(3) && within(days_range, false) : raise
       end
 
     private
@@ -41,18 +33,6 @@ def list_params
       def items_key
         'rows'
       end
-
-      def rescue?(error)
-        bad_request?(error) || backend_error?(error)
-      end
-
-      def bad_request?(error)
-        'badRequest'.in?(error.reasons) && error.message =~ /did not conform/
-      end
-
-      def backend_error?(error)
-        'backendError'.in?(error.reasons)
-      end
     end
   end
 end

lib/yt/models/request.rb

@@ -28,25 +28,13 @@ def initialize(options = {})
       def run
         if response.is_a? @expected_response
           response.tap{|response| response.body = parse_format response.body}
-        elsif run_again_with_refreshed_authentication?
-          run
         else
-          raise error_for(response), request_error_message
+          run_again? ? run : raise(error_for(response), request_error_message)
         end
       end
 
     private
 
-      # If a request authorized with an access token returns 401, then the
-      # access token might have expired. If a refresh token is also present,
-      # try to run the request one more time with a refreshed access token.
-      def run_again_with_refreshed_authentication?
-        if response.is_a? Net::HTTPUnauthorized
-          @response = @http_request = @uri = nil
-          @auth.refresh
-        end if @auth.respond_to? :refresh
-      end
-
       def response
         @response ||= Net::HTTP.start(*net_http_options) do |http|
           http.request http_request
@@ -113,6 +101,45 @@ def parse_format(body)
         end if body
       end
 
+      # There are two cases to run a request again: YouTube responds with a
+      # random error that can be fixed by waiting for some seconds and running
+      # the exact same query, or the access token needs to be refreshed.
+      def run_again?
+        run_again_with_refreshed_authentication? || run_again_after_a_while?
+      end
+
+      # Once in a while, YouTube responds with 500, or 503, or 400 Error and
+      # the text "Invalid query. Query did not conform to the expectations.".
+      # In all these cases, running the same query after some seconds fixes
+      # the issue. This it not documented by YouTube and hardly testable, but
+      # trying again is a workaround that works and hardly causes any damage.
+      def run_again_after_a_while?(max_retries = 1)
+        @retries_so_far ||= -1
+        @retries_so_far += 1
+        if (@retries_so_far < max_retries) && worth_another_try?
+          @response = @http_request = @uri = nil
+          sleep 3
+        end
+      end
+
+      def worth_another_try?
+        case response
+          when Net::HTTPServerError then true
+          when Net::HTTPBadRequest then response.body =~ /did not conform/
+          else false
+        end
+      end
+
+      # If a request authorized with an access token returns 401, then the
+      # access token might have expired. If a refresh token is also present,
+      # try to run the request one more time with a refreshed access token.
+      def run_again_with_refreshed_authentication?
+        if response.is_a? Net::HTTPUnauthorized
+          @response = @http_request = @uri = nil
+          @auth.refresh
+        end if @auth.respond_to? :refresh
+      end
+
       def error_for(response)
         case response
           when Net::HTTPServerError then Errors::ServerError

lib/yt/version.rb

@@ -1,3 +1,3 @@
 module Yt
-  VERSION = '0.5.12'
+  VERSION = '0.5.13'
 end

spec/associations/device_auth/partnered_channels_spec.rb

@@ -7,7 +7,8 @@
     context 'given a content owner with partnered channels' do
       let(:content_owner) { $content_owner }
 
-      it { expect(partnered_channels.count).to be > 0 }
+      # NOTE: Uncomment once size does not runs through *all* the pages
+      # it { expect(partnered_channels.size).to be > 0 }
       it { expect(partnered_channels.first).to be_a Yt::Channel }
     end
   end

spec/collections/earnings_spec.rb

@@ -4,10 +4,8 @@
 describe Yt::Collections::Earnings do
   subject(:collection) { Yt::Collections::Earnings.new parent: channel }
   let(:channel) { Yt::Channel.new id: 'UCxO1tY8h1AhOz0T4ENwmpow' }
-  let(:msg) { {response_body: {error: {errors: [error]}}}.to_json }
   let(:date) { 1.day.ago.to_date }
   let(:dollars) { 10 }
-  let(:message) { 'Invalid query. Query did not conform to the expectations.' }
   before { expect(collection).to behave }
 
   describe '#within' do
@@ -16,43 +14,5 @@
 
       it { expect(collection.within(date..date)[date]).to eq dollars }
     end
-
-    # NOTE: This test is just a reflection of YouTube irrational behavior
-    # of raising 400 or 504 error once in a while when retrieving earnings.
-    # Hopefully this will get fixed and this code (and test) removed.
-    context 'given YouTube responds to the first request with' do
-      let(:behave) { receive(:flat_map) do
-        expect(collection).to receive(:flat_map).and_return [date, dollars]
-        raise Yt::Error, msg
-      end}
-
-      context 'an Invalid Query error' do
-        let(:error) { {reason: 'badRequest', message: message} }
-
-        it { expect(collection.within(date..date)[date]).to eq dollars }
-      end
-
-      context 'a Backend error' do
-        let(:error) { {reason: 'backendError', message: 'Backend Error'} }
-
-        it { expect(collection.within(date..date)[date]).to eq dollars }
-      end
-    end
-
-    context 'given YouTube responds to the second request with' do
-      let(:behave) { receive(:flat_map).twice.and_raise Yt::Error, msg }
-
-      context 'an Invalid Query error' do
-        let(:error) { {reason: 'badRequest', message: message} }
-
-        it { expect{collection.within date..date}.to raise_error Yt::Error }
-      end
-
-      context 'a Backend error' do
-        let(:error) { {reason: 'backendError', message: 'Backend Error'} }
-
-        it { expect{collection.within date..date}.to raise_error Yt::Error }
-      end
-    end
   end
 end

spec/models/request_spec.rb

@@ -1,31 +1,74 @@
 require 'spec_helper'
 require 'yt/models/request'
 
+
 describe Yt::Request do
-  subject(:request) { Yt::Request.new attrs }
-  let(:attrs) { {host: 'example.com'} }
-  before { expect(Net::HTTP).to receive(:start).and_return response }
-  before { allow(response).to receive(:body) }
+  subject(:request) { Yt::Request.new host: 'example.com' }
+  let(:response) { response_class.new nil, nil, nil }
+  let(:response_body) { }
+  before { allow(response).to receive(:body).and_return response_body }
+  before { expect(Net::HTTP).to receive(:start).once.and_return response }
 
   describe '#run' do
-    context 'given a request that returns a 500 code' do
-      let(:response) { Net::HTTPServerError.new nil, nil, nil }
-      it { expect{request.run}.to fail }
-    end
+    context 'given a request that returns' do
+      context 'a success code 2XX' do
+        let(:response_class) { Net::HTTPOK }
 
-    context 'given a request that returns a 401 code' do
-      let(:response) { Net::HTTPUnauthorized.new nil, nil, nil }
-      it { expect{request.run}.to fail }
-    end
+        it { expect{request.run}.not_to fail }
+      end
 
-    context 'given a request that returns a non-2XX code' do
-      let(:response) { Net::HTTPNotFound.new nil, nil, nil }
-      it { expect{request.run}.to fail }
-    end
+      context 'an error code 5XX' do
+        let(:response_class) { Net::HTTPServerError }
+        let(:retry_response) { retry_response_class.new nil, nil, nil }
+        before { allow(retry_response).to receive(:body) }
+        before { expect(Net::HTTP).to receive(:start).at_least(:once).and_return retry_response }
+
+        context 'every time' do
+          let(:retry_response_class) { Net::HTTPServerError }
+
+          it { expect{request.run}.to fail }
+        end
+
+        context 'but returns a success code 2XX the second time' do
+          let(:retry_response_class) { Net::HTTPOK }
+
+          it { expect{request.run}.not_to fail }
+        end
+      end
+
+      context 'an error code 400 with "Invalid Query" message' do
+        let(:response_class) { Net::HTTPBadRequest }
+        let(:response_body) { {error: {errors: [message: message]}}.to_json }
+        let(:message) { 'Invalid query. Query did not conform to the expectations' }
+
+        let(:retry_response) { retry_response_class.new nil, nil, nil }
+        before { allow(retry_response).to receive(:body) }
+        before { expect(Net::HTTP).to receive(:start).at_least(:once).and_return retry_response }
+
+        context 'every time' do
+          let(:retry_response_class) { Net::HTTPBadRequest }
+
+          it { expect{request.run}.to fail }
+        end
+
+        context 'but returns a success code 2XX the second time' do
+          let(:retry_response_class) { Net::HTTPOK }
+
+          it { expect{request.run}.not_to fail }
+        end
+      end
+
+      context 'an error code 401' do
+        let(:response_class) { Net::HTTPUnauthorized }
+
+        it { expect{request.run}.to fail }
+      end
+
+      context 'any other non-2XX error code' do
+        let(:response_class) { Net::HTTPNotFound }
 
-    context 'given a request that returns a 2XX code' do
-      let(:response) { Net::HTTPOK.new nil, nil, nil }
-      it { expect{request.run}.not_to fail }
+        it { expect{request.run}.to fail }
+      end
     end
   end
 end